Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions apps/gittensory-ui/src/components/site/state-views.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import { fireEvent, render, screen } from "@testing-library/react";
import { describe, expect, it, vi } from "vitest";

const { notifyApiFailure } = vi.hoisted(() => ({ notifyApiFailure: vi.fn() }));
vi.mock("@/lib/api/request", () => ({
notifyApiFailure: (...args: unknown[]) => notifyApiFailure(...args),
}));
vi.mock("sonner", () => ({ toast: Object.assign(vi.fn(), { success: vi.fn(), error: vi.fn() }) }));

import { ErrorState, StateBoundary } from "@/components/site/state-views";

describe("ErrorState network-vs-API distinction (#793)", () => {
it("uses the generic 'couldn't load' copy when no errorKind is given (unchanged default)", () => {
render(<ErrorState />);
expect(screen.getByText("Couldn't load this")).toBeTruthy();
});

it("uses connectivity-specific copy for a network errorKind", () => {
render(<ErrorState errorKind="network" />);
expect(screen.getByText("Can't reach the server")).toBeTruthy();
});

it("uses connectivity-specific copy for a timeout errorKind too", () => {
render(<ErrorState errorKind="timeout" />);
expect(screen.getByText("Can't reach the server")).toBeTruthy();
});

it("falls back to the generic copy for an http errorKind", () => {
render(<ErrorState errorKind="http" />);
expect(screen.getByText("Couldn't load this")).toBeTruthy();
});

it("lets an explicit title/description override the errorKind-derived copy", () => {
render(
<ErrorState errorKind="network" title="Custom title" description="Custom description" />,
);
expect(screen.getByText("Custom title")).toBeTruthy();
expect(screen.getByText("Custom description")).toBeTruthy();
expect(screen.queryByText("Can't reach the server")).toBeNull();
});
});

describe("StateBoundary loadingSkeleton (#793)", () => {
it("renders the default spinner LoadingState when no skeleton is given", () => {
render(
<StateBoundary isLoading>
<div>content</div>
</StateBoundary>,
);
expect(screen.getByRole("status")).toBeTruthy();
expect(screen.queryByText("content")).toBeNull();
});

it("renders the provided skeleton instead of the spinner when loading", () => {
render(
<StateBoundary isLoading loadingSkeleton={<div data-testid="skeleton">placeholder</div>}>
<div>content</div>
</StateBoundary>,
);
expect(screen.getByTestId("skeleton")).toBeTruthy();
expect(screen.queryByRole("status")).toBeNull();
expect(screen.queryByText("content")).toBeNull();
});
});

describe("StateBoundary errorKind passthrough (#793)", () => {
it("keeps the pre-#793 default error copy when no errorKind is given", () => {
render(
<StateBoundary isError>
<div>content</div>
</StateBoundary>,
);
expect(screen.getByText("Couldn't load data")).toBeTruthy();
});

it("falls through to ErrorState's network-aware copy when errorKind is network and no override is given", () => {
render(
<StateBoundary isError errorKind="network">
<div>content</div>
</StateBoundary>,
);
expect(screen.getByText("Can't reach the server")).toBeTruthy();
expect(screen.queryByText("Couldn't load data")).toBeNull();
});

it("lets an explicit errorTitle/errorDescription win even with a network errorKind", () => {
render(
<StateBoundary
isError
errorKind="network"
errorTitle="Custom title"
errorDescription="Custom description"
>
<div>content</div>
</StateBoundary>,
);
expect(screen.getByText("Custom title")).toBeTruthy();
expect(screen.getByText("Custom description")).toBeTruthy();
});

it("passes the real errorKind (not a hardcoded 'network') to the error-failure notifier", () => {
render(
<StateBoundary isError errorKind="http" errorLabel="Widgets">
<div>content</div>
</StateBoundary>,
);
expect(notifyApiFailure).toHaveBeenCalledWith(expect.objectContaining({ kind: "http" }));
});

it("defaults the notifier kind to 'network' when no errorKind is given, matching pre-#793 behavior", () => {
render(
<StateBoundary isError errorLabel="Widgets">
<div>content</div>
</StateBoundary>,
);
expect(notifyApiFailure).toHaveBeenCalledWith(expect.objectContaining({ kind: "network" }));
});
});

describe("StateBoundary retry/refresh actions (#793 regression guard)", () => {
it("still invokes onRetry from the error state's retry button", () => {
const onRetry = vi.fn();
render(
<StateBoundary isError onRetry={onRetry}>
<div>content</div>
</StateBoundary>,
);
fireEvent.click(screen.getByRole("button", { name: /try again/i }));
expect(onRetry).toHaveBeenCalledTimes(1);
});

it("renders children unchanged when neither loading, error, nor empty", () => {
render(
<StateBoundary>
<div>content</div>
</StateBoundary>,
);
expect(screen.getByText("content")).toBeTruthy();
});
});
73 changes: 58 additions & 15 deletions apps/gittensory-ui/src/components/site/state-views.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import { Loader2, Inbox, AlertTriangle, RefreshCw } from "lucide-react";
import { Loader2, Inbox, AlertTriangle, RefreshCw, WifiOff } from "lucide-react";
import { useCallback, useEffect, useState, type ReactNode } from "react";
import { toast } from "sonner";

import { cn } from "@/lib/utils";
import { notifyApiFailure } from "@/lib/api/request";
import { notifyApiFailure, type ApiFailureKind } from "@/lib/api/request";

/** `errorKind`s that mean "we never reached the server" as opposed to "the server answered with an
* error" — StateBoundary/ErrorState use this to show connectivity-specific copy and iconography (#793). */
const NETWORK_ERROR_KINDS: ReadonlySet<ApiFailureKind> = new Set(["network", "timeout"]);

/**
* Shared state primitives so every panel/route renders a consistent
Expand Down Expand Up @@ -140,8 +144,9 @@ export function StateActionButton({
}

export function ErrorState({
title = "Couldn't load this",
description = "Something went wrong fetching this data. You can retry, or come back in a moment. If this keeps happening, check status or try again shortly.",
title,
description,
errorKind,
onRetry,
retryLabel = "Try again",
secondaryAction,
Expand All @@ -150,18 +155,34 @@ export function ErrorState({
}: {
title?: string;
description?: ReactNode;
/** Distinguishes "couldn't reach the server at all" from "the server answered with an error" (#793).
* Only changes the default `title`/`description` — an explicit `title`/`description` always wins. */
errorKind?: ApiFailureKind;
onRetry?: () => void;
retryLabel?: string;
secondaryAction?: ReactNode;
toastOnRetry?: boolean;
className?: string;
}) {
const isNetworkIssue = errorKind !== undefined && NETWORK_ERROR_KINDS.has(errorKind);
const resolvedTitle = title ?? (isNetworkIssue ? "Can't reach the server" : "Couldn't load this");
const resolvedDescription =
description ??
(isNetworkIssue
? "We couldn't reach the API — check your connection and retry. This is a connectivity issue, not a problem with the data itself."
: "Something went wrong fetching this data. You can retry, or come back in a moment. If this keeps happening, check status or try again shortly.");
return (
<Shell
role="alert"
icon={<AlertTriangle className="size-5 text-warning" aria-hidden />}
title={title}
description={description}
icon={
isNetworkIssue ? (
<WifiOff className="size-5 text-warning" aria-hidden />
) : (
<AlertTriangle className="size-5 text-warning" aria-hidden />
)
}
title={resolvedTitle}
description={resolvedDescription}
action={
onRetry && (
<StateActionButton
Expand Down Expand Up @@ -218,10 +239,12 @@ export function StateBoundary({
isEmpty,
loadingTitle = "Loading data…",
loadingDescription = "Fetching the latest available signals for this view.",
loadingSkeleton,
emptyTitle = "No data available yet",
emptyDescription = "This view has no records to show. Refresh when the source data is available.",
errorTitle = "Couldn't load data",
errorDescription = "The data source did not respond. Retry the request, or check back once the service has recovered.",
errorTitle,
errorDescription,
errorKind,
onRetry,
onRefresh,
errorLabel,
Expand All @@ -232,38 +255,58 @@ export function StateBoundary({
isEmpty?: boolean;
loadingTitle?: string;
loadingDescription?: ReactNode;
/** Content-shaped placeholder (build with `@/components/ui/skeleton`) shown instead of the generic
* spinner while loading, so the layout doesn't jump once data arrives (#793). */
loadingSkeleton?: ReactNode;
emptyTitle?: string;
emptyDescription?: ReactNode;
errorTitle?: string;
errorDescription?: ReactNode;
/** Distinguishes "couldn't reach the server at all" from "the server answered with an error" (#793). */
errorKind?: ApiFailureKind;
onRetry?: () => void;
onRefresh?: () => void;
/** When provided, surfaces a global API-failure toast with a Retry action. */
errorLabel?: string;
children: ReactNode;
}) {
const isNetworkIssue = errorKind !== undefined && NETWORK_ERROR_KINDS.has(errorKind);
// Preserve the pre-#793 defaults exactly when no errorKind is given; when one is given and the caller
// didn't override the copy, fall through to ErrorState's own network-aware defaults instead.
const resolvedErrorTitle = errorTitle ?? (isNetworkIssue ? undefined : "Couldn't load data");
const resolvedErrorDescription =
errorDescription ??
(isNetworkIssue
? undefined
: "The data source did not respond. Retry the request, or check back once the service has recovered.");

// When this boundary flips into the error state, surface a toast with Retry.
useEffect(() => {
if (isError && errorLabel) {
notifyApiFailure({
label: errorLabel,
kind: "network",
kind: errorKind ?? "network",
message:
typeof errorDescription === "string" ? errorDescription : "Data source did not respond.",
typeof resolvedErrorDescription === "string"
? resolvedErrorDescription
: "Data source did not respond.",
retry: onRetry,
});
}
}, [isError, errorLabel, errorDescription, onRetry]);
}, [isError, errorLabel, errorKind, resolvedErrorDescription, onRetry]);

if (isLoading) {
return <LoadingState title={loadingTitle} description={loadingDescription} />;
return (
loadingSkeleton ?? <LoadingState title={loadingTitle} description={loadingDescription} />
);
}

if (isError) {
return (
<ErrorState
title={errorTitle}
description={errorDescription}
title={resolvedErrorTitle}
description={resolvedErrorDescription}
errorKind={errorKind}
onRetry={onRetry}
toastOnRetry={false}
secondaryAction={
Expand Down
50 changes: 50 additions & 0 deletions apps/gittensory-ui/src/lib/api/use-api-resource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,53 @@ describe("useApiResource loadedAt (#2219)", () => {
expect(result.current.loadedAt).toBeNull();
});
});

describe("useApiResource errorKind/errorStatus (#793)", () => {
it("carries the apiFetch failure kind and status through to the error state", async () => {
apiFetch.mockResolvedValue({
ok: false,
kind: "http",
message: "500 Internal Server Error",
status: 500,
durationMs: 5,
});
const { result } = renderHook(() => useApiResource("/v1/thing", "Thing"));
await waitFor(() => expect(result.current.status).toBe("error"));
expect(result.current).toMatchObject({ errorKind: "http", errorStatus: 500 });
});

it("carries a network failure kind with no status", async () => {
apiFetch.mockResolvedValue({
ok: false,
kind: "network",
message: "fetch failed",
durationMs: 5,
});
const { result } = renderHook(() => useApiResource("/v1/thing", "Thing"));
await waitFor(() => expect(result.current.status).toBe("error"));
expect(result.current).toMatchObject({ errorKind: "network", errorStatus: undefined });
});

it("carries a timeout failure kind", async () => {
apiFetch.mockResolvedValue({
ok: false,
kind: "timeout",
message: "Request timed out",
durationMs: 5,
});
const { result } = renderHook(() => useApiResource("/v1/thing", "Thing"));
await waitFor(() => expect(result.current.status).toBe("error"));
expect(result.current).toMatchObject({ errorKind: "timeout" });
});

it("leaves errorKind undefined for the synthetic disabled sentinel", async () => {
const { result } = renderHook(() =>
useApiResource("/v1/thing", "Thing", undefined, { enabled: false }),
);
await waitFor(() => expect(result.current.status).toBe("error"));
const state = result.current;
if (state.status !== "error") throw new Error("expected error status");
expect(state.error).toBe("disabled");
expect(state.errorKind).toBeUndefined();
});
});
21 changes: 18 additions & 3 deletions apps/gittensory-ui/src/lib/api/use-api-resource.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
import { useCallback, useEffect, useState } from "react";

import { getApiOrigin } from "./origin";
import { apiFetch } from "./request";
import { apiFetch, type ApiFailureKind } from "./request";

type ResourceState<T> =
| { status: "loading"; data: null; error: null; loadedAt: null }
| { status: "ready"; data: T; error: null; loadedAt: number }
| { status: "error"; data: null; error: string; loadedAt: null };
| {
status: "error";
data: null;
error: string;
/** Absent for the synthetic "disabled" sentinel below — only real `apiFetch` failures carry one (#793). */
errorKind?: ApiFailureKind;
errorStatus?: number;
loadedAt: null;
};

type UseApiResourceOptions = {
enabled?: boolean;
Expand Down Expand Up @@ -42,7 +50,14 @@ export function useApiResource<T>(
if (result.ok) {
setState({ status: "ready", data: result.data, error: null, loadedAt: Date.now() });
} else {
setState({ status: "error", data: null, error: result.message, loadedAt: null });
setState({
status: "error",
data: null,
error: result.message,
errorKind: result.kind,
errorStatus: result.status,
loadedAt: null,
});
}
}, [enabled, label, path, token]);

Expand Down
Loading
Loading