Surface setup error state instead of console.warn in useSetup
This commit is contained in:
@@ -484,6 +484,8 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue.
|
||||
|
||||
**Goal:** Replace the `console.warn` with `setError(err instanceof Error ? err.message : "Unknown error")` so that the error is surfaced to the consuming component via the hook's public interface. Remove the `console.warn` entirely — the hook already has an `error` state for this purpose.
|
||||
|
||||
**Status:** Completed.
|
||||
|
||||
**Possible traps and issues:**
|
||||
- Verify that `SetupPage.tsx` (or whatever consumes `useSetup`) already renders the `error` state. If it does not, add an error message display (a Fluent UI `<MessageBar intent="error">`) so the surfaced error is actually visible to the user.
|
||||
- `console.warn` in hooks that ship to production will produce console noise for end users and may appear in error monitoring tools as unexpected warnings. The rule is to handle errors in state, not in the console.
|
||||
|
||||
@@ -47,9 +47,6 @@ export function useSetup(): UseSetupResult {
|
||||
} catch (err: unknown) {
|
||||
const fallback = "Failed to fetch setup status";
|
||||
handleFetchError(err, setError, fallback);
|
||||
if (!(err instanceof DOMException && err.name === "AbortError")) {
|
||||
console.warn("Setup status check failed:", err instanceof Error ? err.message : fallback);
|
||||
}
|
||||
} finally {
|
||||
setLoading(false);
|
||||
}
|
||||
|
||||
@@ -67,9 +67,7 @@ describe("SetupPage", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("renders the form and logs a warning when the status check fails", async () => {
|
||||
// Task 0.4: catch block must log a warning and keep the form visible.
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
it("renders the form and surfaces the error message when the status check fails", async () => {
|
||||
mockedGetSetupStatus.mockRejectedValue(new Error("Connection refused"));
|
||||
renderPage();
|
||||
await waitFor(() => {
|
||||
@@ -77,7 +75,6 @@ describe("SetupPage", () => {
|
||||
screen.getByRole("heading", { name: /bangui setup/i }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
expect(warnSpy).toHaveBeenCalledOnce();
|
||||
warnSpy.mockRestore();
|
||||
expect(screen.getByText(/connection refused/i)).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user