From 8904e180d109b8ffcf9dcf974bac87ba926eb34e Mon Sep 17 00:00:00 2001
From: Lukas <lukas.pupkalipinski@lpl-mind.de>
Date: Thu, 23 Apr 2026 09:45:11 +0200
Subject: [PATCH] Fix: Prevent session-expiry errors from briefly showing in
 useConfigItem.save()

When save() encounters a 401 or 403 error, the HTTP client dispatches
SESSION_EXPIRED_EVENT which triggers auth handling and navigation to login.
However, setSaveError was called first, causing a brief flash of an
'Unauthorized' message before the redirect.

Now, isAuthError(err) checks if the error is a 401/403 before setting
saveError. Auth errors are rethrown without setting error state, allowing
the auth handler to deal with session expiry cleanly without UX confusion.

- Import isAuthError from api/client in useConfigItem hook
- Check for auth errors in the save() catch block before setSaveError
- Add tests for 401 and 403 error handling

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 Docs/Tasks.md                                 | 20 ----------
 .../src/hooks/__tests__/useConfigItem.test.ts | 37 +++++++++++++++++++
 frontend/src/hooks/useConfigItem.ts           |  2 +
 3 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/Docs/Tasks.md b/Docs/Tasks.md
index 444903b..a6d6990 100644
--- a/Docs/Tasks.md
+++ b/Docs/Tasks.md
@@ -1,23 +1,3 @@
-### TASK-QUALITY-01 — `KVEditor` Uses `entryKeys.join(",")` as Effect Dependency
-
-**Where found**
-`frontend/src/components/config/KVEditor.tsx`. An effect dependency is computed as `entryKeys.join(",")`. This works for most key values but produces incorrect results (false equality) when any key contains a comma character — two different key sets could produce the same joined string.
-
-**Goal**
-Replace the join-based comparison with a stable serialisation that cannot produce false equality. The simplest correct option is `JSON.stringify(entryKeys)`, which handles commas, empty strings, and special characters correctly. Alternatively, use `useDeepCompareEffect` from a utility library, or maintain a counter that increments whenever keys change.
-
-**Possible traps and issues**
-- `JSON.stringify` on a large array is marginally more expensive than `join`. For a config editor with typically fewer than 50 keys this cost is negligible.
-- Ensure the dependency is the full keys array (not the joined string) and let React's referential equality handle the common case; only reach for `JSON.stringify` if the array reference itself is not stable.
-
-**Docs changes needed**
-None required.
-
-**Why this is needed**
-A KV entry key containing a comma (e.g. `"a,b"` vs separate keys `"a"` and `"b"`) would cause the effect to not fire when it should, silently failing to update derived state.
-
----
-
 ### TASK-QUALITY-02 — `useConfigItem.save()` Briefly Shows Session-Expiry as Save Error
 
 **Where found**
diff --git a/frontend/src/hooks/__tests__/useConfigItem.test.ts b/frontend/src/hooks/__tests__/useConfigItem.test.ts
index 39876a5..f8bfbdb 100644
--- a/frontend/src/hooks/__tests__/useConfigItem.test.ts
+++ b/frontend/src/hooks/__tests__/useConfigItem.test.ts
@@ -1,6 +1,7 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 import { renderHook, act } from "@testing-library/react";
 import { useConfigItem } from "../useConfigItem";
+import { ApiError } from "../../api/client";
 
 describe("useConfigItem", () => {
   beforeEach(() => {
@@ -85,4 +86,40 @@ describe("useConfigItem", () => {
 
     expect(result.current.saveError).toBe("save failed");
   });
+
+  it("auth errors are rethrown without setting saveError", async () => {
+    const fetchFn = vi.fn().mockResolvedValue("ok");
+    const authError = new ApiError(401, "Unauthorized");
+    const saveFn = vi.fn().mockRejectedValue(authError);
+
+    const { result } = renderHook(() => useConfigItem<string, string>({ fetchFn, saveFn }));
+
+    await act(async () => {
+      await Promise.resolve();
+    });
+
+    await act(async () => {
+      await expect(result.current.save("test")).rejects.toThrow(authError);
+    });
+
+    expect(result.current.saveError).toBeNull();
+  });
+
+  it("403 errors are rethrown without setting saveError", async () => {
+    const fetchFn = vi.fn().mockResolvedValue("ok");
+    const forbiddenError = new ApiError(403, "Forbidden");
+    const saveFn = vi.fn().mockRejectedValue(forbiddenError);
+
+    const { result } = renderHook(() => useConfigItem<string, string>({ fetchFn, saveFn }));
+
+    await act(async () => {
+      await Promise.resolve();
+    });
+
+    await act(async () => {
+      await expect(result.current.save("test")).rejects.toThrow(forbiddenError);
+    });
+
+    expect(result.current.saveError).toBeNull();
+  });
 });
diff --git a/frontend/src/hooks/useConfigItem.ts b/frontend/src/hooks/useConfigItem.ts
index 1916d50..a292d52 100644
--- a/frontend/src/hooks/useConfigItem.ts
+++ b/frontend/src/hooks/useConfigItem.ts
@@ -3,6 +3,7 @@
  */
 import { useCallback, useEffect, useRef, useState } from "react";
 import { handleFetchError } from "../utils/fetchError";
+import { isAuthError } from "../api/client";
 
 export interface UseConfigItemResult<T, U> {
   data: T | null;
@@ -71,6 +72,7 @@ export function useConfigItem<T, U>(
           setData((prevData) => mergeOnSave(prevData, update));
         }
       } catch (err: unknown) {
+        if (isAuthError(err)) throw err; // let auth handler deal with it
         const message = err instanceof Error ? err.message : "Failed to save data";
         setSaveError(message);
         throw err;