From 51c6887d04a0e00003136552089c0c54fd2f41b7 Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Mon, 26 Oct 2026 01:00:57 -0700 Subject: [PATCH 1/5] fix(frontend): ErrorState with requestId on dashboard pages (SEL-410) Alerts page uses ErrorState with real refetch; request IDs from x-request-id flow to ErrorState on eight dashboard pages; records page copy per SEL-409 frontend. Signed-off-by: Christopher Sellers <1@christophersellers.com> --- frontend/src/__tests__/lib/api-error.test.ts | 31 ++++++++- .../pages/dashboard-load-errors.test.tsx | 49 +++++++++++++- .../pages/sel409-load-errors.test.tsx | 9 ++- frontend/src/app/dashboard/alerts/page.tsx | 26 +++----- frontend/src/app/dashboard/audit-log/page.tsx | 5 +- .../src/app/dashboard/compliance/page.tsx | 3 +- .../src/app/dashboard/notifications/page.tsx | 5 +- frontend/src/app/dashboard/products/page.tsx | 5 +- frontend/src/app/dashboard/qa-health/page.tsx | 3 +- .../src/app/dashboard/recall-report/page.tsx | 3 +- frontend/src/app/dashboard/settings/page.tsx | 5 +- frontend/src/app/records/page.tsx | 20 ++---- .../dashboard/agent-review-dashboard.tsx | 3 +- .../components/dashboard/curator-review.tsx | 3 +- frontend/src/hooks/use-agent-reviews.ts | 3 +- frontend/src/hooks/use-api-query.ts | 4 +- frontend/src/hooks/use-api.ts | 4 +- frontend/src/hooks/use-billing.ts | 5 +- frontend/src/hooks/use-control-plane.ts | 3 +- frontend/src/hooks/use-review.ts | 4 +- frontend/src/lib/api-error.ts | 66 ++++++++++++++++++- frontend/src/lib/api-hooks.ts | 6 +- 22 files changed, 203 insertions(+), 62 deletions(-) diff --git a/frontend/src/__tests__/lib/api-error.test.ts b/frontend/src/__tests__/lib/api-error.test.ts index 2faa919ed..91563e8ed 100644 --- a/frontend/src/__tests__/lib/api-error.test.ts +++ b/frontend/src/__tests__/lib/api-error.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { apiError, getErrorStatus, isAuthError } from '@/lib/api-error'; +import { apiError, getErrorRequestId, getErrorStatus, isAuthError, requestIdFromResponse, withRequestId } from '@/lib/api-error'; describe('api-error helpers', () => { it('builds errors with machine-readable status in both fields', () => { @@ -17,6 +17,35 @@ describe('api-error helpers', () => { expect(getErrorStatus({ status: '401' })).toBeNull(); }); + it('attaches a request ID to the direct and nested error shape', () => { + const err = apiError(500, 'Internal Server Error', 'req-1'); + expect(err.requestId).toBe('req-1'); + expect(err.response?.requestId).toBe('req-1'); + expect(getErrorRequestId(err)).toBe('req-1'); + expect(getErrorRequestId(apiError(500, ''))).toBeUndefined(); + }); + + it('reads the request ID from the failed response header', () => { + const res = new Response('boom', { status: 500, headers: { 'x-request-id': 'req-2' } }); + expect(requestIdFromResponse(res)).toBe('req-2'); + expect(requestIdFromResponse(new Response('boom', { status: 500 }))).toBeUndefined(); + expect(requestIdFromResponse(null)).toBeUndefined(); + expect(requestIdFromResponse({ ok: false, status: 500 })).toBeUndefined(); + expect(requestIdFromResponse({ headers: { 'x-request-id': 'req-plain' } })).toBe('req-plain'); + + const err = withRequestId(new Error('nope'), res); + expect(err.message).toBe('nope'); + expect(getErrorRequestId(err)).toBe('req-2'); + }); + + it('reads the request ID from API client header and plain shapes', () => { + expect(getErrorRequestId({ response: { status: 500, headers: { 'x-request-id': 'req-3' } } })).toBe('req-3'); + expect(getErrorRequestId({ requestId: 'req-4' })).toBe('req-4'); + expect(getErrorRequestId(new Error('plain'))).toBeUndefined(); + expect(getErrorRequestId(null)).toBeUndefined(); + expect(getErrorRequestId({ requestId: 42 })).toBeUndefined(); + }); + it('flags only 401 and 403 as auth errors', () => { expect(isAuthError(apiError(401, ''))).toBe(true); expect(isAuthError(apiError(403, ''))).toBe(true); diff --git a/frontend/src/__tests__/pages/dashboard-load-errors.test.tsx b/frontend/src/__tests__/pages/dashboard-load-errors.test.tsx index f60926616..c7761c235 100644 --- a/frontend/src/__tests__/pages/dashboard-load-errors.test.tsx +++ b/frontend/src/__tests__/pages/dashboard-load-errors.test.tsx @@ -1,8 +1,10 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { ErrorState } from '@/components/ui/error-state'; +import { apiError } from '@/lib/api-error'; +import AlertsPage from '@/app/dashboard/alerts/page'; import ProductsPage from '@/app/dashboard/products/page'; import AuditLogPage from '@/app/dashboard/audit-log/page'; import NotificationsPage from '@/app/dashboard/notifications/page'; @@ -23,6 +25,7 @@ const useSubscriptionMock = vi.hoisted(() => vi.fn()); const fetchComplianceScoreMock = vi.hoisted(() => vi.fn()); const fetchWorkbenchSummaryMock = vi.hoisted(() => vi.fn()); const fetchRecallReportMock = vi.hoisted(() => vi.fn()); +const fetchAlertsMock = vi.hoisted(() => vi.fn()); const tenantMock = vi.hoisted(() => ({ tenantId: 't' as string | null })); vi.mock('@/lib/fetch-with-csrf', () => ({ fetchWithCsrf: (...args: unknown[]) => fetchWithCsrfMock(...args) })); @@ -40,6 +43,8 @@ vi.mock('@/lib/api-hooks', () => ({ fetchComplianceScore: () => fetchComplianceScoreMock(), fetchWorkbenchReadinessSummary: () => fetchWorkbenchSummaryMock(), fetchRecallReport: () => fetchRecallReportMock(), + fetchAlerts: (...args: unknown[]) => fetchAlertsMock(...args), + acknowledgeAlert: vi.fn(), })); vi.mock('@/hooks/use-agent-reviews', () => ({ useAgentReviewItems: () => useReviewItemsMock(), @@ -59,11 +64,14 @@ function renderWithQuery(ui: React.ReactElement) { const failedResponse = Response.error(); +afterEach(() => vi.unstubAllEnvs()); + beforeEach(() => { fetchWithCsrfMock.mockReset(); fetchComplianceScoreMock.mockReset(); fetchWorkbenchSummaryMock.mockReset(); fetchRecallReportMock.mockReset(); + fetchAlertsMock.mockReset(); tenantMock.tenantId = 't'; authMock.mockReturnValue({ isAuthenticated: true, hasTenantSession: true, hasAdminSession: true, isHydrated: true, user: { is_sysadmin: false } }); fetchWorkbenchSummaryMock.mockResolvedValue(null); @@ -91,6 +99,45 @@ describe('shared dashboard load errors', () => { expect(screen.queryByText(/Request ID:/)).not.toBeInTheDocument(); }); + it('shows the alerts error state with retry, support, and the request ID', async () => { + vi.stubEnv('NEXT_PUBLIC_ENABLE_EXPERIMENTAL_FEATURES', 'true'); + fetchAlertsMock.mockRejectedValue(apiError(500, 'Internal Server Error', 'alerts-req-1')); + const user = userEvent.setup(); + renderWithQuery(); + + expect(await screen.findByRole('alert')).toHaveTextContent('We could not load your alerts.'); + expect(screen.getByText('Request ID: alerts-req-1')).toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Contact support' })).toHaveAttribute('href', '/settings/support'); + await user.click(screen.getByRole('button', { name: 'Retry' })); + await waitFor(() => expect(fetchAlertsMock).toHaveBeenCalledTimes(2)); + }); + + it('shows the failed response request ID on the products error state', async () => { + fetchWithCsrfMock.mockResolvedValue( + new Response(JSON.stringify({ detail: 'boom' }), { + status: 500, + headers: { 'x-request-id': 'products-req-1' }, + }), + ); + renderWithQuery(); + + expect(await screen.findByRole('alert')).toHaveTextContent('We could not load this page.'); + expect(screen.getByText('Request ID: products-req-1')).toBeInTheDocument(); + }); + + it('shows the API client request ID on QA health failures', () => { + const refetch = vi.fn(); + authMock.mockReturnValue({ user: { is_sysadmin: true }, isHydrated: true }); + const qaError = Object.assign(new Error('Request failed'), { + response: { status: 500, headers: { 'x-request-id': 'qa-req-9' }, data: {} }, + }); + useQAHealthMock.mockReturnValue({ data: null, isLoading: false, error: qaError, refetch, isRefetching: false }); + render(); + + expect(screen.getByRole('alert')).toHaveTextContent('We could not load this page.'); + expect(screen.getByText('Request ID: qa-req-9')).toBeInTheDocument(); + }); + it('keeps Add Product unavailable until a failed catalog request succeeds on Retry', async () => { fetchWithCsrfMock .mockResolvedValueOnce(failedResponse) diff --git a/frontend/src/__tests__/pages/sel409-load-errors.test.tsx b/frontend/src/__tests__/pages/sel409-load-errors.test.tsx index 4bcc767b9..f9fb42255 100644 --- a/frontend/src/__tests__/pages/sel409-load-errors.test.tsx +++ b/frontend/src/__tests__/pages/sel409-load-errors.test.tsx @@ -36,10 +36,15 @@ vi.mock('@/hooks/use-control-plane', () => ({ })); describe('SEL-409 failed workspace loads', () => { - it('hides the records count', () => { + it('hides the records count behind the records error state', () => { + failed.refetch.mockClear(); render(); - expect(screen.getByRole('alert')).toHaveTextContent('Load failed'); + expect(screen.getByRole('alert')).toHaveTextContent('We could not load your records.'); expect(screen.queryByText(/total records/i)).not.toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Contact support' })).toHaveAttribute('href', '/settings/support'); + expect(screen.queryByText(/Request ID:/)).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Retry' })); + expect(failed.refetch).toHaveBeenCalledOnce(); }); it('hides the rules count', () => { diff --git a/frontend/src/app/dashboard/alerts/page.tsx b/frontend/src/app/dashboard/alerts/page.tsx index d80ce7ce1..236d77ef0 100644 --- a/frontend/src/app/dashboard/alerts/page.tsx +++ b/frontend/src/app/dashboard/alerts/page.tsx @@ -12,6 +12,7 @@ import { Button } from '@/components/ui/button'; import { Callout } from '@/components/ui/callout'; import { Card } from '@/components/ui/card'; import { EmptyState } from '@/components/ui/empty-state'; +import { ErrorState } from '@/components/ui/error-state'; import { StatTile } from '@/components/ui/stat-tile'; import { Spinner } from '@/components/ui/spinner'; import { cn } from '@/lib/utils'; @@ -34,6 +35,7 @@ import { import { useAuth } from '@/lib/auth-context'; import { useTenant } from '@/lib/tenant-context'; +import { getErrorRequestId } from '@/lib/api-error'; import { fetchAlerts, acknowledgeAlert } from '@/lib/api-hooks'; /* ── Types matching AlertsResponse from backend ── */ @@ -119,7 +121,7 @@ function AlertsDashboardPageContent() { const [filter, setFilter] = useState('all'); - const { data: alertsData, isLoading: loading, error: alertsError, refetch: loadAlerts, isSuccess } = useQuery({ + const { data: alertsData, isLoading: loading, error: alertsError, refetch: loadAlerts, isFetching, isSuccess } = useQuery({ queryKey: ['alerts', tenantId], queryFn: async () => { const data = (await fetchAlerts(tenantId)) as AlertsResponse; @@ -201,22 +203,12 @@ function AlertsDashboardPageContent() { {/* Error */} {fetchFailed && ( - loadAlerts()} disabled={loading}> - + loadAlerts()} + retrying={isFetching} + requestId={getErrorRequestId(alertsError)} + /> )} {isLoggedIn && !loading && alerts.length > 0 && ( diff --git a/frontend/src/app/dashboard/audit-log/page.tsx b/frontend/src/app/dashboard/audit-log/page.tsx index ec8cd3c81..19e5c0571 100644 --- a/frontend/src/app/dashboard/audit-log/page.tsx +++ b/frontend/src/app/dashboard/audit-log/page.tsx @@ -37,6 +37,7 @@ import { import Link from 'next/link'; import { useAuth } from '@/lib/auth-context'; import { fetchWithCsrf } from '@/lib/fetch-with-csrf'; +import { getErrorRequestId, withRequestId } from '@/lib/api-error'; import { useTenant } from '@/lib/tenant-context'; import { useDashboardRefresh } from '@/hooks/use-dashboard-refresh'; @@ -116,7 +117,7 @@ async function fetchAuditLog(tenantId: string, page = 1, pageSize = 50): Promise }); if (!res.ok) { console.error(`Audit log fetch failed: ${res.status} ${res.statusText}`); - throw new Error("Couldn't load the audit log."); + throw withRequestId(new Error("Couldn't load the audit log."), res); } return res.json(); } @@ -244,7 +245,7 @@ export default function AuditLogPage() { )} {/* Error */} - {error && loadLog()} />} + {error && loadLog()} requestId={getErrorRequestId(error)} />} {/* Event list shell — toolbar, rows, pagination */} {isLoggedIn && !error && entries.length > 0 && ( diff --git a/frontend/src/app/dashboard/compliance/page.tsx b/frontend/src/app/dashboard/compliance/page.tsx index 0ee189b69..fe90da7a7 100644 --- a/frontend/src/app/dashboard/compliance/page.tsx +++ b/frontend/src/app/dashboard/compliance/page.tsx @@ -32,6 +32,7 @@ import { import { useAuth } from '@/lib/auth-context'; import { useTenant } from '@/lib/tenant-context'; import { fetchComplianceScore, fetchWorkbenchReadinessSummary } from '@/lib/api-hooks'; +import { getErrorRequestId } from '@/lib/api-error'; import { ErrorState } from '@/components/ui/error-state'; import { filterExperimentalLinks } from '@/lib/experimental-features'; import { NO_DATA_YET } from '@/lib/empty-values'; @@ -457,7 +458,7 @@ export default function ComplianceDashboardPage() { {/* Error */} {error && ( - { fetchScore(); fetchWorkbenchSummary(); }} /> + { fetchScore(); fetchWorkbenchSummary(); }} requestId={getErrorRequestId(error)} /> )} {/* Score Content */} diff --git a/frontend/src/app/dashboard/notifications/page.tsx b/frontend/src/app/dashboard/notifications/page.tsx index dada2edc4..095acdf57 100644 --- a/frontend/src/app/dashboard/notifications/page.tsx +++ b/frontend/src/app/dashboard/notifications/page.tsx @@ -24,6 +24,7 @@ import { import { useAuth } from '@/lib/auth-context'; import { useTenant } from '@/lib/tenant-context'; +import { getErrorRequestId, withRequestId } from '@/lib/api-error'; /* ── Types matching NotificationPreferences ── */ @@ -80,7 +81,7 @@ async function apiFetchPrefs(tenantId: string): Promise credentials: 'include', headers: { 'Content-Type': 'application/json' }, }); - if (!res.ok) throw new Error(`API error: ${res.status} ${res.statusText}`); + if (!res.ok) throw withRequestId(new Error(`API error: ${res.status} ${res.statusText}`), res); return res.json(); } @@ -243,7 +244,7 @@ export default function NotificationPrefsPage() {
)} - {fetchFailed && refetchPrefs()} retrying={isFetching} />} + {fetchFailed && refetchPrefs()} retrying={isFetching} requestId={getErrorRequestId(prefsError)} />} {saveError && ( diff --git a/frontend/src/app/dashboard/products/page.tsx b/frontend/src/app/dashboard/products/page.tsx index c379901c8..76d3ef920 100644 --- a/frontend/src/app/dashboard/products/page.tsx +++ b/frontend/src/app/dashboard/products/page.tsx @@ -33,6 +33,7 @@ import { import { useAuth } from '@/lib/auth-context'; import { useTenant } from '@/lib/tenant-context'; +import { getErrorRequestId, withRequestId } from '@/lib/api-error'; import { useDashboardRefresh } from '@/hooks/use-dashboard-refresh'; /* ── Types matching ProductCatalogResponse ── */ @@ -78,7 +79,7 @@ async function apiFetchProducts(tenantId: string): Promise )} - {error && loadProducts()} />} + {error && loadProducts()} requestId={getErrorRequestId(error)} />} {isLoggedIn && !error && products.length > 0 && ( <> diff --git a/frontend/src/app/dashboard/qa-health/page.tsx b/frontend/src/app/dashboard/qa-health/page.tsx index f8c00dd0c..2e31560bc 100644 --- a/frontend/src/app/dashboard/qa-health/page.tsx +++ b/frontend/src/app/dashboard/qa-health/page.tsx @@ -24,6 +24,7 @@ import { StatusBadge, type ComplianceState } from "@/components/ui/status-badge" import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; import { useQAHealth } from "@/hooks/use-api"; import { useAuth } from "@/lib/auth-context"; +import { getErrorRequestId } from "@/lib/api-error"; import { isRequiresStepUpPayload, requestStepUp } from "@/lib/step-up-events"; import type { QAHealthArtifactRef, @@ -295,7 +296,7 @@ export default function QAHealthPage() { return ( - refetch()} retrying={isRefetching} /> + refetch()} retrying={isRefetching} requestId={getErrorRequestId(error)} /> ); } diff --git a/frontend/src/app/dashboard/recall-report/page.tsx b/frontend/src/app/dashboard/recall-report/page.tsx index e90f181be..e6d03e155 100644 --- a/frontend/src/app/dashboard/recall-report/page.tsx +++ b/frontend/src/app/dashboard/recall-report/page.tsx @@ -13,6 +13,7 @@ import { Skeleton } from '@/components/ui/skeleton'; import { useAuth } from '@/lib/auth-context'; import { useTenant } from '@/lib/tenant-context'; import { fetchRecallReport } from '@/lib/api-hooks'; +import { getErrorRequestId } from '@/lib/api-error'; import { ErrorState } from '@/components/ui/error-state'; import { fetchWithCsrf } from '@/lib/fetch-with-csrf'; import { @@ -120,7 +121,7 @@ export default function RecallReportPage() {
- refetch()} /> + refetch()} requestId={getErrorRequestId(error)} />
); diff --git a/frontend/src/app/dashboard/settings/page.tsx b/frontend/src/app/dashboard/settings/page.tsx index 503ae28d6..da897ff58 100644 --- a/frontend/src/app/dashboard/settings/page.tsx +++ b/frontend/src/app/dashboard/settings/page.tsx @@ -20,6 +20,7 @@ import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'; import { useAuth } from '@/lib/auth-context'; import { useTenant } from '@/lib/tenant-context'; import { useCurrentSubscription } from '@/hooks/use-billing'; +import { getErrorRequestId } from '@/lib/api-error'; import { Building2, Key, Database, Plug, Save, CheckCircle2, CreditCard, @@ -105,7 +106,7 @@ export default function SettingsPage() { const { user, isAuthenticated } = useAuth(); const { tenantId, isSystemTenant } = useTenant(); const effectiveTenantId = isSystemTenant ? null : tenantId; - const { data: subscriptionData, isLoading: subLoading, isError: subError, refetch: refetchSubscription, isFetching: subFetching } = useCurrentSubscription(effectiveTenantId ?? undefined); + const { data: subscriptionData, isLoading: subLoading, isError: subError, error: subQueryError, refetch: refetchSubscription, isFetching: subFetching } = useCurrentSubscription(effectiveTenantId ?? undefined); const [saved, setSaved] = useState(false); const [activeTab, setActiveTab] = useState<'profile' | 'api' | 'retention' | 'integrations'>('profile'); @@ -271,7 +272,7 @@ export default function SettingsPage() { ) : subError ? (
{planName} - refetchSubscription()} retrying={subFetching} className="mt-2" /> + refetchSubscription()} retrying={subFetching} requestId={getErrorRequestId(subQueryError)} className="mt-2" />
) : ( <> diff --git a/frontend/src/app/records/page.tsx b/frontend/src/app/records/page.tsx index 463509df2..d984ad470 100644 --- a/frontend/src/app/records/page.tsx +++ b/frontend/src/app/records/page.tsx @@ -7,15 +7,16 @@ import { PageHeader } from '@/components/layout/page-header'; import { Section } from '@/components/layout/section'; import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; -import { Callout } from '@/components/ui/callout'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { DataTable, type DataTableColumn } from '@/components/ui/data-table'; +import { ErrorState } from '@/components/ui/error-state'; import { fieldControlClasses } from '@/components/ui/input'; import { Skeleton } from '@/components/ui/skeleton'; import { StatusBadge, type ComplianceState } from '@/components/ui/status-badge'; import { cn } from '@/lib/utils'; import { useAuth } from '@/lib/auth-context'; +import { getErrorRequestId } from '@/lib/api-error'; import { useCanonicalEvents, useCanonicalEvent, @@ -91,18 +92,11 @@ export default function CanonicalRecordsPage() { return ( {pageHeader} - events.refetch()}> - Retry - - } - > - {(events.error as Error).message} - + events.refetch()} + requestId={getErrorRequestId(events.error)} + /> ); } diff --git a/frontend/src/components/dashboard/agent-review-dashboard.tsx b/frontend/src/components/dashboard/agent-review-dashboard.tsx index ac44fd258..ae9548beb 100644 --- a/frontend/src/components/dashboard/agent-review-dashboard.tsx +++ b/frontend/src/components/dashboard/agent-review-dashboard.tsx @@ -15,6 +15,7 @@ import { } from 'lucide-react'; import { useAgentReviewDecision, useAgentReviewItems } from '@/hooks/use-agent-reviews'; +import { getErrorRequestId } from '@/lib/api-error'; import { useAuth } from '@/lib/auth-context'; import { useTenant } from '@/lib/tenant-context'; import { cn } from '@/lib/utils'; @@ -636,7 +637,7 @@ export function AgentReviewDashboard() {

Loading agent reviews...

) : query.error ? ( - query.refetch()} retrying={query.isFetching} className="m-4" /> + query.refetch()} retrying={query.isFetching} requestId={getErrorRequestId(query.error)} className="m-4" /> ) : sortedItems.length === 0 ? ( refetch()} retrying={isFetching} />; + return refetch()} retrying={isFetching} requestId={getErrorRequestId(error)} />; } if (sortedItems.length === 0) { diff --git a/frontend/src/hooks/use-agent-reviews.ts b/frontend/src/hooks/use-agent-reviews.ts index 44d6d23fe..4fb0f078d 100644 --- a/frontend/src/hooks/use-agent-reviews.ts +++ b/frontend/src/hooks/use-agent-reviews.ts @@ -3,6 +3,7 @@ import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; import { fetchWithCsrf } from '@/lib/fetch-with-csrf'; +import { withRequestId } from '@/lib/api-error'; import type { AgentReviewDecisionInput, AgentReviewDecisionResponse, @@ -53,7 +54,7 @@ async function parseAgentReviewError(response: Response): Promise { : typeof errorBody?.detail === 'string' ? errorBody.detail : `Agent review request failed: ${response.status} ${response.statusText}`; - return new Error(message); + return withRequestId(new Error(message), response); } async function readJsonResponse(response: Response): Promise { diff --git a/frontend/src/hooks/use-api-query.ts b/frontend/src/hooks/use-api-query.ts index ca5892cdf..e2e80e1be 100644 --- a/frontend/src/hooks/use-api-query.ts +++ b/frontend/src/hooks/use-api-query.ts @@ -20,7 +20,7 @@ import { useAuth } from '@/lib/auth-context'; import { useTenant } from '@/lib/tenant-context'; import { getServiceURL } from '@/lib/api-config'; import { fetchWithCsrf } from '@/lib/fetch-with-csrf'; -import { apiError } from '@/lib/api-error'; +import { apiError, requestIdFromResponse } from '@/lib/api-error'; type ServiceName = 'ingestion' | 'admin' | 'compliance' | 'graph'; @@ -46,7 +46,7 @@ async function authFetch( if (!res.ok) { // Keep the status machine-readable so callers can tell 401 (session // expired, redirect to login) from 403 (forbidden) without parsing text. - throw apiError(res.status, res.statusText); + throw apiError(res.status, res.statusText, requestIdFromResponse(res)); } return res.json(); diff --git a/frontend/src/hooks/use-api.ts b/frontend/src/hooks/use-api.ts index 73071e107..5de21680c 100644 --- a/frontend/src/hooks/use-api.ts +++ b/frontend/src/hooks/use-api.ts @@ -1,6 +1,6 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { apiClient } from '@/lib/api-client'; -import { apiError, isAuthError } from '@/lib/api-error'; +import { apiError, isAuthError, requestIdFromResponse } from '@/lib/api-error'; import type { IngestURLRequest } from '@/types/api'; import type { LabelBatchInitRequest } from '@/types/labels'; import type { SystemStatusResponse, SystemMetricsResponse } from '@/lib/api-client'; @@ -60,7 +60,7 @@ async function fetchAggregatedStatus(): Promise { // down. Tag the status so the caller can surface it instead of // reporting healthy-looking "unhealthy" demo services. if (res.status === 401 || res.status === 403) { - throw apiError(res.status, res.statusText); + throw apiError(res.status, res.statusText, requestIdFromResponse(res)); } throw new Error(`HTTP ${res.status}`); } diff --git a/frontend/src/hooks/use-billing.ts b/frontend/src/hooks/use-billing.ts index b9e9f21ff..1fb4737b7 100644 --- a/frontend/src/hooks/use-billing.ts +++ b/frontend/src/hooks/use-billing.ts @@ -7,6 +7,7 @@ import { useMutation, useQuery } from '@tanstack/react-query'; import { getServiceURL } from '@/lib/api-config'; +import { withRequestId } from '@/lib/api-error'; import { fetchWithCsrf } from '@/lib/fetch-with-csrf'; // Legacy billing helpers still route through the admin proxy when those @@ -28,7 +29,7 @@ async function billingFetch(path: string, options?: RequestInit): Promise if (!res.ok) { const error = await res.json().catch(() => ({ detail: 'Request failed' })); - throw new Error(error.detail || `Billing API error: ${res.status}`); + throw withRequestId(new Error(error.detail || `Billing API error: ${res.status}`), res); } return res.json(); @@ -191,7 +192,7 @@ async function fetchCurrentSubscription(tenantId: string): Promise ({ detail: 'Request failed' })); - throw new Error(error.detail || `Billing API error: ${res.status}`); + throw withRequestId(new Error(error.detail || `Billing API error: ${res.status}`), res); } return res.json(); diff --git a/frontend/src/hooks/use-control-plane.ts b/frontend/src/hooks/use-control-plane.ts index 2146a423a..4b7392627 100644 --- a/frontend/src/hooks/use-control-plane.ts +++ b/frontend/src/hooks/use-control-plane.ts @@ -3,6 +3,7 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { useAuth } from '@/lib/auth-context'; import { fetchWithCsrf } from '@/lib/fetch-with-csrf'; +import { withRequestId } from '@/lib/api-error'; import { POLL_CONTROL_PLANE_MS, POLL_DATA_MS, POLL_METRICS_MS } from '@/lib/polling-config'; // --------------------------------------------------------------------------- @@ -42,7 +43,7 @@ async function cpFetch( // Backend errors arrive as {detail} (FastAPI HTTPException) or as the // monolith envelope {error: {type, message}} — read both so callers' // toasts show the domain message instead of a bare "HTTP 400". - throw new Error(error.detail || error?.error?.message || `HTTP ${response.status}`); + throw withRequestId(new Error(error.detail || error?.error?.message || `HTTP ${response.status}`), response); } const data = await response.json(); diff --git a/frontend/src/hooks/use-review.ts b/frontend/src/hooks/use-review.ts index 7fa286a46..adcce2424 100644 --- a/frontend/src/hooks/use-review.ts +++ b/frontend/src/hooks/use-review.ts @@ -2,7 +2,7 @@ import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; import { fetchWithCsrf } from '@/lib/fetch-with-csrf'; -import { apiError } from '@/lib/api-error'; +import { apiError, requestIdFromResponse } from '@/lib/api-error'; import type { ReviewItem } from '@/types/review'; async function reviewFetch(path: string, options: RequestInit = {}): Promise { @@ -23,7 +23,7 @@ async function reviewFetch(path: string, options: RequestInit = {}): Promise< : null; // Keep the status machine-readable alongside the server message so a // 401 on the review queue can trigger re-login instead of a dead list. - const error = apiError(response.status, response.statusText); + const error = apiError(response.status, response.statusText, requestIdFromResponse(response)); if (serverMessage) error.message = serverMessage; throw error; } diff --git a/frontend/src/lib/api-error.ts b/frontend/src/lib/api-error.ts index e1e0b6cad..3cd0c7ceb 100644 --- a/frontend/src/lib/api-error.ts +++ b/frontend/src/lib/api-error.ts @@ -14,15 +14,51 @@ export type StatusError = Error & { status?: number; - response?: { status?: number; data?: unknown }; + requestId?: string; + response?: { status?: number; data?: unknown; headers?: Record; requestId?: string }; }; /** Build an error that keeps the HTTP status machine-readable. */ -export function apiError(status: number, statusText: string): StatusError { +export function apiError(status: number, statusText: string, requestId?: string | null): StatusError { const suffix = statusText ? ` ${statusText}` : ''; const error = new Error(`API error: ${status}${suffix}`) as StatusError; error.status = status; error.response = { status }; + if (requestId) { + error.requestId = requestId; + error.response.requestId = requestId; + } + return error; +} + +/** + * Read the request ID the backend stamped on a failed response. The shared + * request-id middleware sets X-Request-ID on every response, so this is real + * correlation data for support, not a client invention. Returns undefined when + * the response carries none (network failure, opaque error). + */ +export function requestIdFromResponse(res: unknown): string | undefined { + if (!res || typeof res !== 'object') return undefined; + const headers = (res as { headers?: unknown }).headers; + if (!headers) return undefined; + if (typeof (headers as { get?: unknown }).get === 'function') { + const value = (headers as { get: (name: string) => string | null }).get('x-request-id'); + return value || undefined; + } + if (typeof headers === 'object') { + const value = (headers as Record)['x-request-id'] + ?? (headers as Record)['X-Request-ID']; + return typeof value === 'string' && value ? value : undefined; + } + return undefined; +} + +/** Attach the failed response's request ID to any error, preserving its type. */ +export function withRequestId(error: T, res: unknown): T { + const requestId = requestIdFromResponse(res); + if (requestId) { + (error as StatusError).requestId = requestId; + } return error; } @@ -43,6 +79,32 @@ export function getErrorStatus(error: unknown): number | null { return null; } +/** Read the request ID off any error shape our fetch layers produce. */ +export function getErrorRequestId(error: unknown): string | undefined { + if (!error || typeof error !== 'object') return undefined; + const record = error as { requestId?: unknown; response?: unknown }; + if (typeof record.requestId === 'string' && record.requestId) { + return record.requestId; + } + const response = record.response; + if (response && typeof response === 'object') { + const nested = response as { requestId?: unknown; headers?: unknown }; + if (typeof nested.requestId === 'string' && nested.requestId) { + return nested.requestId; + } + // APIClient stores lowercased response headers on error.response. + const headers = nested.headers; + if (headers && typeof headers === 'object') { + const map = headers as Record; + const value = map['x-request-id'] ?? map['X-Request-ID']; + if (typeof value === 'string' && value) { + return value; + } + } + } + return undefined; +} + /** True when the error is an authentication or authorization failure. */ export function isAuthError(error: unknown): boolean { const status = getErrorStatus(error); diff --git a/frontend/src/lib/api-hooks.ts b/frontend/src/lib/api-hooks.ts index 9ed0afc29..185c1c406 100644 --- a/frontend/src/lib/api-hooks.ts +++ b/frontend/src/lib/api-hooks.ts @@ -10,7 +10,7 @@ import { getServiceURL } from './api-config'; import { CSRF_PROTECTED_METHODS } from './csrf'; import { fetchWithCsrf } from './fetch-with-csrf'; -import { apiError } from './api-error'; +import { apiError, requestIdFromResponse } from './api-error'; const BASE = () => getServiceURL('ingestion'); const API_FETCH_TIMEOUT_MS = 8000; @@ -103,14 +103,14 @@ async function apiFetch(path: string, _apiKey?: string | null, options: Reque // sit outside the try/catch above so they are never mistaken for a // retryable transport failure. if (res.status === 429 || (res.status >= 500 && res.status <= 599)) { - lastError = apiError(res.status, res.statusText); + lastError = apiError(res.status, res.statusText, requestIdFromResponse(res)); if (attempt < MAX_RETRIES) { await new Promise(resolve => setTimeout(resolve, RETRY_DELAYS[attempt])); } continue; } - throw apiError(res.status, res.statusText); + throw apiError(res.status, res.statusText, requestIdFromResponse(res)); } throw lastError!; From 0a3ce7ec7ecba3fd8338a0b95756c03dc58fdec1 Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Mon, 26 Oct 2026 01:46:35 -0700 Subject: [PATCH 2/5] fix(frontend): pass x-request-id through proxies and use generated test IDs Addresses Codex review on #3621: allowlist x-request-id, keep it on 5xx proxy rebuilds and review/items responses, replace invented test IDs with crypto.randomUUID(). Signed-off-by: Christopher Sellers <1@christophersellers.com> --- .../api/privileged-session-routes.test.ts | 54 ++++++++++++ .../__tests__/api/proxy-sanitization.test.ts | 87 +++++++++++++++++++ frontend/src/__tests__/lib/api-error.test.ts | 25 +++--- .../pages/dashboard-load-errors.test.tsx | 15 ++-- frontend/src/app/api/review/items/route.ts | 8 +- frontend/src/lib/api-proxy.ts | 5 +- frontend/src/lib/proxy-factory.ts | 9 +- 7 files changed, 182 insertions(+), 21 deletions(-) diff --git a/frontend/src/__tests__/api/privileged-session-routes.test.ts b/frontend/src/__tests__/api/privileged-session-routes.test.ts index a1b98a403..75527db7b 100644 --- a/frontend/src/__tests__/api/privileged-session-routes.test.ts +++ b/frontend/src/__tests__/api/privileged-session-routes.test.ts @@ -112,6 +112,60 @@ describe('privileged frontend session forwarding', () => { assertOpaqueSessionOnly(fetchMock.mock.calls[0][1]); }); + it('copies the upstream x-request-id onto review queue error responses', async () => { + const requestId = crypto.randomUUID(); + fetchMock.mockResolvedValueOnce(new Response(JSON.stringify({ detail: 'boom' }), { + status: 500, + headers: { 'content-type': 'application/json', 'x-request-id': requestId }, + })); + + const response = await getReviewItems( + browserRequest('https://app.regengine.test/api/review/items'), + ); + + expect(response.status).toBe(500); + expect(response.headers.get('x-request-id')).toBe(requestId); + expect(await response.json()).toEqual({ error: 'boom' }); + }); + + it('copies the upstream x-request-id onto transformed review queue success responses', async () => { + const requestId = crypto.randomUUID(); + fetchMock.mockResolvedValueOnce(new Response(JSON.stringify({ + items: [{ + review_id: '6ba7b810-9dad-41d1-a0b4-00c04fd430c8', + doc_hash: 'abc123', + confidence_score: 0.42, + created_at: '2026-10-01T00:00:00Z', + updated_at: '2026-10-02T00:00:00Z', + status: 'PENDING', + tenant_id: 'tenant-123', + text_preview: 'preview text', + extraction: { lot: 'L1' }, + }], + }), { + status: 200, + headers: { 'content-type': 'application/json', 'x-request-id': requestId }, + })); + + const response = await getReviewItems( + browserRequest('https://app.regengine.test/api/review/items'), + ); + + expect(response.status).toBe(200); + expect(response.headers.get('x-request-id')).toBe(requestId); + expect(await response.json()).toEqual([{ + id: '6ba7b810-9dad-41d1-a0b4-00c04fd430c8', + doc_hash: 'abc123', + confidence_score: 0.42, + created_at: '2026-10-01T00:00:00Z', + updated_at: '2026-10-02T00:00:00Z', + status: 'PENDING', + tenant_id: 'tenant-123', + source_text: 'preview text', + extracted_data: { lot: 'L1' }, + }]); + }); + it('sends no reviewer identity or retired credential on a decision', async () => { fetchMock.mockResolvedValueOnce(new Response(JSON.stringify({ status: 'approved', diff --git a/frontend/src/__tests__/api/proxy-sanitization.test.ts b/frontend/src/__tests__/api/proxy-sanitization.test.ts index 0295ea2f2..fd95a265f 100644 --- a/frontend/src/__tests__/api/proxy-sanitization.test.ts +++ b/frontend/src/__tests__/api/proxy-sanitization.test.ts @@ -416,6 +416,93 @@ describe('proxy 5xx error sanitization', () => { }); }); +describe('proxy x-request-id passthrough', () => { + it('passes the upstream x-request-id through on stream proxy success', async () => { + const requestId = crypto.randomUUID(); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'content-type': 'application/json', 'x-request-id': requestId }, + }))); + const handlers = createStreamProxy({ + serviceName: 'ingestion', + resolveTargetBases: () => ['http://internal.service.local'], + buildHeaders: () => new Headers(), + methods: ['GET'], + }); + + const response = await handlers.GET!( + makeRouteRequest(), + { params: Promise.resolve({ path: ['jobs'] }) }, + ); + + expect(response.status).toBe(200); + expect(response.headers.get('x-request-id')).toBe(requestId); + }); + + it('preserves the upstream x-request-id on rebuilt stream proxy 5xx responses', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + const requestId = crypto.randomUUID(); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response('boom', { + status: 500, + headers: { 'content-type': 'text/plain', 'x-request-id': requestId }, + }))); + const handlers = createStreamProxy({ + serviceName: 'admin', + resolveTargetBases: () => ['http://internal.service.local'], + buildHeaders: () => new Headers(), + methods: ['GET'], + }); + + const response = await handlers.GET!( + makeRouteRequest(), + { params: Promise.resolve({ path: ['users'] }) }, + ); + + expect(response.status).toBe(500); + expect(response.headers.get('x-request-id')).toBe(requestId); + expect(errorSpy).toHaveBeenCalled(); + }); + + it('preserves the upstream x-request-id on rebuilt JSON proxy 5xx responses', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + const requestId = crypto.randomUUID(); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(JSON.stringify({ + detail: 'boom', + }), { + status: 503, + headers: { 'content-type': 'application/json', 'x-request-id': requestId }, + }))); + const handlers = createJsonProxy({ + serviceName: 'compliance', + buildTargetUrl: () => 'http://internal.service.local/compliance/check', + buildHeaders: () => new Headers(), + methods: ['GET'], + }); + + const response = await handlers.GET!( + makeRouteRequest(), + { params: Promise.resolve({ path: ['check'] }) }, + ); + + expect(response.status).toBe(503); + expect(response.headers.get('x-request-id')).toBe(requestId); + const body = await responseJson(response); + expect(body).toEqual({ error: 'Service temporarily unavailable' }); + expect(errorSpy).toHaveBeenCalled(); + }); + + it('sets x-request-id on proxyError responses only when a request ID is provided', () => { + const requestId = crypto.randomUUID(); + + const withId = proxyError('admin request failed', 503, { requestId }); + expect(withId.status).toBe(503); + expect(withId.headers.get('x-request-id')).toBe(requestId); + + const withoutId = proxyError('admin request failed', 503); + expect(withoutId.headers.get('x-request-id')).toBeNull(); + }); +}); + describe('requireProxyAuth', () => { it('does not treat REGENGINE_API_KEY as a caller credential', () => { configureFirstPartyServiceCredential(); diff --git a/frontend/src/__tests__/lib/api-error.test.ts b/frontend/src/__tests__/lib/api-error.test.ts index 91563e8ed..4951b36e3 100644 --- a/frontend/src/__tests__/lib/api-error.test.ts +++ b/frontend/src/__tests__/lib/api-error.test.ts @@ -18,29 +18,34 @@ describe('api-error helpers', () => { }); it('attaches a request ID to the direct and nested error shape', () => { - const err = apiError(500, 'Internal Server Error', 'req-1'); - expect(err.requestId).toBe('req-1'); - expect(err.response?.requestId).toBe('req-1'); - expect(getErrorRequestId(err)).toBe('req-1'); + const requestId = crypto.randomUUID(); + const err = apiError(500, 'Internal Server Error', requestId); + expect(err.requestId).toBe(requestId); + expect(err.response?.requestId).toBe(requestId); + expect(getErrorRequestId(err)).toBe(requestId); expect(getErrorRequestId(apiError(500, ''))).toBeUndefined(); }); it('reads the request ID from the failed response header', () => { - const res = new Response('boom', { status: 500, headers: { 'x-request-id': 'req-2' } }); - expect(requestIdFromResponse(res)).toBe('req-2'); + const requestId = crypto.randomUUID(); + const plainRequestId = crypto.randomUUID(); + const res = new Response('boom', { status: 500, headers: { 'x-request-id': requestId } }); + expect(requestIdFromResponse(res)).toBe(requestId); expect(requestIdFromResponse(new Response('boom', { status: 500 }))).toBeUndefined(); expect(requestIdFromResponse(null)).toBeUndefined(); expect(requestIdFromResponse({ ok: false, status: 500 })).toBeUndefined(); - expect(requestIdFromResponse({ headers: { 'x-request-id': 'req-plain' } })).toBe('req-plain'); + expect(requestIdFromResponse({ headers: { 'x-request-id': plainRequestId } })).toBe(plainRequestId); const err = withRequestId(new Error('nope'), res); expect(err.message).toBe('nope'); - expect(getErrorRequestId(err)).toBe('req-2'); + expect(getErrorRequestId(err)).toBe(requestId); }); it('reads the request ID from API client header and plain shapes', () => { - expect(getErrorRequestId({ response: { status: 500, headers: { 'x-request-id': 'req-3' } } })).toBe('req-3'); - expect(getErrorRequestId({ requestId: 'req-4' })).toBe('req-4'); + const headerRequestId = crypto.randomUUID(); + const directRequestId = crypto.randomUUID(); + expect(getErrorRequestId({ response: { status: 500, headers: { 'x-request-id': headerRequestId } } })).toBe(headerRequestId); + expect(getErrorRequestId({ requestId: directRequestId })).toBe(directRequestId); expect(getErrorRequestId(new Error('plain'))).toBeUndefined(); expect(getErrorRequestId(null)).toBeUndefined(); expect(getErrorRequestId({ requestId: 42 })).toBeUndefined(); diff --git a/frontend/src/__tests__/pages/dashboard-load-errors.test.tsx b/frontend/src/__tests__/pages/dashboard-load-errors.test.tsx index c7761c235..e7e3d297a 100644 --- a/frontend/src/__tests__/pages/dashboard-load-errors.test.tsx +++ b/frontend/src/__tests__/pages/dashboard-load-errors.test.tsx @@ -101,41 +101,44 @@ describe('shared dashboard load errors', () => { it('shows the alerts error state with retry, support, and the request ID', async () => { vi.stubEnv('NEXT_PUBLIC_ENABLE_EXPERIMENTAL_FEATURES', 'true'); - fetchAlertsMock.mockRejectedValue(apiError(500, 'Internal Server Error', 'alerts-req-1')); + const alertsRequestId = crypto.randomUUID(); + fetchAlertsMock.mockRejectedValue(apiError(500, 'Internal Server Error', alertsRequestId)); const user = userEvent.setup(); renderWithQuery(); expect(await screen.findByRole('alert')).toHaveTextContent('We could not load your alerts.'); - expect(screen.getByText('Request ID: alerts-req-1')).toBeInTheDocument(); + expect(screen.getByText(`Request ID: ${alertsRequestId}`)).toBeInTheDocument(); expect(screen.getByRole('link', { name: 'Contact support' })).toHaveAttribute('href', '/settings/support'); await user.click(screen.getByRole('button', { name: 'Retry' })); await waitFor(() => expect(fetchAlertsMock).toHaveBeenCalledTimes(2)); }); it('shows the failed response request ID on the products error state', async () => { + const productsRequestId = crypto.randomUUID(); fetchWithCsrfMock.mockResolvedValue( new Response(JSON.stringify({ detail: 'boom' }), { status: 500, - headers: { 'x-request-id': 'products-req-1' }, + headers: { 'x-request-id': productsRequestId }, }), ); renderWithQuery(); expect(await screen.findByRole('alert')).toHaveTextContent('We could not load this page.'); - expect(screen.getByText('Request ID: products-req-1')).toBeInTheDocument(); + expect(screen.getByText(`Request ID: ${productsRequestId}`)).toBeInTheDocument(); }); it('shows the API client request ID on QA health failures', () => { const refetch = vi.fn(); authMock.mockReturnValue({ user: { is_sysadmin: true }, isHydrated: true }); + const qaRequestId = crypto.randomUUID(); const qaError = Object.assign(new Error('Request failed'), { - response: { status: 500, headers: { 'x-request-id': 'qa-req-9' }, data: {} }, + response: { status: 500, headers: { 'x-request-id': qaRequestId }, data: {} }, }); useQAHealthMock.mockReturnValue({ data: null, isLoading: false, error: qaError, refetch, isRefetching: false }); render(); expect(screen.getByRole('alert')).toHaveTextContent('We could not load this page.'); - expect(screen.getByText('Request ID: qa-req-9')).toBeInTheDocument(); + expect(screen.getByText(`Request ID: ${qaRequestId}`)).toBeInTheDocument(); }); it('keeps Add Product unavailable until a failed catalog request succeeds on Retry', async () => { diff --git a/frontend/src/app/api/review/items/route.ts b/frontend/src/app/api/review/items/route.ts index 2b48c7d6a..653dca20f 100644 --- a/frontend/src/app/api/review/items/route.ts +++ b/frontend/src/app/api/review/items/route.ts @@ -38,9 +38,10 @@ export async function GET(request: NextRequest) { if (!res.ok) { const error = await res.json().catch(() => ({})); + const requestId = res.headers.get('x-request-id'); return NextResponse.json( { error: error.detail || 'Failed to load review queue' }, - { status: res.status }, + requestId ? { status: res.status, headers: { 'x-request-id': requestId } } : { status: res.status }, ); } @@ -70,7 +71,10 @@ export async function GET(request: NextRequest) { extracted_data: item.extraction || {}, })); - return NextResponse.json(items); + const requestId = res.headers.get('x-request-id'); + return requestId + ? NextResponse.json(items, { headers: { 'x-request-id': requestId } }) + : NextResponse.json(items); } catch (error: unknown) { console.error('Review queue fetch failed:', error); diff --git a/frontend/src/lib/api-proxy.ts b/frontend/src/lib/api-proxy.ts index 5ffd7befd..3a430262c 100644 --- a/frontend/src/lib/api-proxy.ts +++ b/frontend/src/lib/api-proxy.ts @@ -220,7 +220,7 @@ export function genericProxyErrorMessage(status: number): string { export function proxyError( message: string, status: number, - opts?: { code?: string; details?: string[] }, + opts?: { code?: string; details?: string[]; requestId?: string }, ): NextResponse { const serverError = isServerErrorStatus(status); const body: ProxyErrorBody = { @@ -228,6 +228,9 @@ export function proxyError( }; if (!serverError && opts?.code) body.code = opts.code; if (!serverError && opts?.details) body.details = opts.details; + if (opts?.requestId) { + return NextResponse.json(body, { status, headers: { 'x-request-id': opts.requestId } }); + } return NextResponse.json(body, { status }); } diff --git a/frontend/src/lib/proxy-factory.ts b/frontend/src/lib/proxy-factory.ts index db73aacdb..ce5ec9eb5 100644 --- a/frontend/src/lib/proxy-factory.ts +++ b/frontend/src/lib/proxy-factory.ts @@ -183,7 +183,9 @@ export function createJsonProxy(config: JsonProxyConfig): MethodHandlers { targetUrl, detail: data, }); - return proxyError(`${config.serviceName} request failed`, response.status); + return proxyError(`${config.serviceName} request failed`, response.status, { + requestId: response.headers.get('x-request-id') ?? undefined, + }); } return NextResponse.json( { error: detail ?? `${config.serviceName} request failed` }, @@ -248,6 +250,7 @@ const DEFAULT_STREAM_RESPONSE_HEADERS = [ 'content-disposition', 'cache-control', 'x-fda-record-count', + 'x-request-id', ]; const AUTH_BROWSER_NONCE_COOKIE = 'regengine_auth_browser_nonce'; const OPAQUE_SESSION_COOKIE = '__Host-regengine_session'; @@ -417,7 +420,9 @@ export function createStreamProxy(config: StreamProxyConfig): MethodHandlers { targetUrl, upstreamError: await readUpstreamErrorForLog(response), }); - return proxyError(`${config.serviceName} request failed`, response.status); + return proxyError(`${config.serviceName} request failed`, response.status, { + requestId: response.headers.get('x-request-id') ?? undefined, + }); } const outgoing = new Headers(); From 4173c509fbf0533b8764557a0516c58973c15322 Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Mon, 26 Oct 2026 01:49:55 -0700 Subject: [PATCH 3/5] fix(frontend): show safe rate-limit guidance on records load 429 Signed-off-by: Christopher Sellers <1@christophersellers.com> --- .../pages/sel410-records-load-errors.test.tsx | 75 +++++++++++++++++++ frontend/src/app/records/page.tsx | 16 +++- frontend/src/hooks/use-control-plane.ts | 18 ++++- frontend/src/lib/api-error.ts | 39 ++++++++++ 4 files changed, 144 insertions(+), 4 deletions(-) create mode 100644 frontend/src/__tests__/pages/sel410-records-load-errors.test.tsx diff --git a/frontend/src/__tests__/pages/sel410-records-load-errors.test.tsx b/frontend/src/__tests__/pages/sel410-records-load-errors.test.tsx new file mode 100644 index 000000000..25efbea07 --- /dev/null +++ b/frontend/src/__tests__/pages/sel410-records-load-errors.test.tsx @@ -0,0 +1,75 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest'; +import { fireEvent, render, screen } from '@testing-library/react'; + +import RecordsPage from '@/app/records/page'; +import { apiError } from '@/lib/api-error'; + +const state = vi.hoisted(() => ({ error: null as unknown })); +const refetch = vi.fn(); + +vi.mock('@/lib/auth-context', () => ({ + useAuth: () => ({ tenantId: '', user: null }), +})); +vi.mock('@/hooks/use-control-plane', () => ({ + useCanonicalEvents: () => ({ data: undefined, error: state.error, isLoading: false, refetch }), + useCanonicalEvent: () => ({ data: undefined }), +})); + +describe('SEL-410 records load errors', () => { + beforeEach(() => { + refetch.mockClear(); + }); + + it('shows the safe rate-limit message on 429 without Retry-After', () => { + const requestId = crypto.randomUUID(); + const rawDetail = `rate-limit-detail-${crypto.randomUUID()}`; + state.error = apiError(429, rawDetail, requestId); + + render(); + + expect(screen.getByRole('alert')).toHaveTextContent('Too many requests. Wait a moment before retrying.'); + expect(screen.queryByText(rawDetail)).not.toBeInTheDocument(); + expect(screen.queryByText(/total records/i)).not.toBeInTheDocument(); + expect(screen.getByText(`Request ID: ${requestId}`)).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Retry' })); + expect(refetch).toHaveBeenCalledOnce(); + }); + + it('mentions the Retry-After seconds on 429 when the header shape carries them', () => { + const requestId = crypto.randomUUID(); + const rawDetail = `rate-limit-detail-${crypto.randomUUID()}`; + const error = apiError(429, rawDetail, requestId); + error.response = { status: 429, headers: { 'retry-after': '45' }, requestId }; + state.error = error; + + render(); + + const alert = screen.getByRole('alert'); + expect(alert).toHaveTextContent('Too many requests. Wait a moment before retrying.'); + expect(alert).toHaveTextContent('Retry in 45 seconds.'); + expect(screen.queryByText(rawDetail)).not.toBeInTheDocument(); + }); + + it('mentions the Retry-After seconds on 429 when the direct field carries them', () => { + const error = apiError(429, 'Too Many Requests'); + error.retryAfter = 30; + state.error = error; + + render(); + + expect(screen.getByRole('alert')).toHaveTextContent('Retry in 30 seconds.'); + }); + + it('keeps the generic message for non-429 failures and hides raw detail', () => { + const rawDetail = `backend-detail-${crypto.randomUUID()}`; + state.error = apiError(500, rawDetail); + + render(); + + expect(screen.getByRole('alert')).toHaveTextContent('We could not load your records.'); + expect(screen.queryByText(rawDetail)).not.toBeInTheDocument(); + expect(screen.queryByText(/total records/i)).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Retry' })); + expect(refetch).toHaveBeenCalledOnce(); + }); +}); diff --git a/frontend/src/app/records/page.tsx b/frontend/src/app/records/page.tsx index d984ad470..34949ab20 100644 --- a/frontend/src/app/records/page.tsx +++ b/frontend/src/app/records/page.tsx @@ -16,7 +16,7 @@ import { StatusBadge, type ComplianceState } from '@/components/ui/status-badge' import { cn } from '@/lib/utils'; import { useAuth } from '@/lib/auth-context'; -import { getErrorRequestId } from '@/lib/api-error'; +import { getErrorRequestId, getErrorRetryAfter, getErrorStatus } from '@/lib/api-error'; import { useCanonicalEvents, useCanonicalEvent, @@ -51,6 +51,14 @@ const RESULT_STATE: Record = { const FILTER_SELECT = 'h-8 w-auto max-w-[13rem] py-0 pr-8 text-small'; +/** Safe 429 copy; appends the Retry-After wait only when it is a plain second count. */ +function rateLimitMessage(retryAfterSeconds: number | null): string { + if (retryAfterSeconds === null) { + return 'Too many requests. Wait a moment before retrying.'; + } + return `Too many requests. Wait a moment before retrying. Retry in ${retryAfterSeconds} seconds.`; +} + /** Key/value row for the record detail
— label above value, hairline separated. */ function DetailRow({ term, children }: { term: string; children: React.ReactNode }) { return ( @@ -89,11 +97,15 @@ export default function CanonicalRecordsPage() { if (events.error) { + // Fixed safe copy only — the raw backend detail is never rendered. + const loadErrorMessage = getErrorStatus(events.error) === 429 + ? rateLimitMessage(getErrorRetryAfter(events.error)) + : 'We could not load your records.'; return ( {pageHeader} events.refetch()} requestId={getErrorRequestId(events.error)} /> diff --git a/frontend/src/hooks/use-control-plane.ts b/frontend/src/hooks/use-control-plane.ts index 4b7392627..d36012cb4 100644 --- a/frontend/src/hooks/use-control-plane.ts +++ b/frontend/src/hooks/use-control-plane.ts @@ -3,7 +3,7 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { useAuth } from '@/lib/auth-context'; import { fetchWithCsrf } from '@/lib/fetch-with-csrf'; -import { withRequestId } from '@/lib/api-error'; +import { withRequestId, type StatusError } from '@/lib/api-error'; import { POLL_CONTROL_PLANE_MS, POLL_DATA_MS, POLL_METRICS_MS } from '@/lib/polling-config'; // --------------------------------------------------------------------------- @@ -43,7 +43,21 @@ async function cpFetch( // Backend errors arrive as {detail} (FastAPI HTTPException) or as the // monolith envelope {error: {type, message}} — read both so callers' // toasts show the domain message instead of a bare "HTTP 400". - throw withRequestId(new Error(error.detail || error?.error?.message || `HTTP ${response.status}`), response); + const thrown = withRequestId( + new Error(error.detail || error?.error?.message || `HTTP ${response.status}`), + response, + ) as StatusError; + // Keep the status machine-readable so pages can branch on it (e.g. 429 + // rate-limit messaging) via getErrorStatus. + thrown.status = response.status; + const retryAfterHeader = response.headers?.get?.('retry-after'); + if (retryAfterHeader != null && retryAfterHeader.trim() !== '') { + const seconds = Number(retryAfterHeader.trim()); + if (Number.isInteger(seconds) && seconds >= 0) { + thrown.retryAfter = seconds; + } + } + throw thrown; } const data = await response.json(); diff --git a/frontend/src/lib/api-error.ts b/frontend/src/lib/api-error.ts index 3cd0c7ceb..24fc9f9c0 100644 --- a/frontend/src/lib/api-error.ts +++ b/frontend/src/lib/api-error.ts @@ -15,6 +15,7 @@ export type StatusError = Error & { status?: number; requestId?: string; + retryAfter?: number; response?: { status?: number; data?: unknown; headers?: Record; requestId?: string }; }; @@ -105,6 +106,44 @@ export function getErrorRequestId(error: unknown): string | undefined { return undefined; } +function toRetryAfterSeconds(value: unknown): number | null { + if (typeof value === 'number') { + return Number.isInteger(value) && value >= 0 ? value : null; + } + if (typeof value === 'string' && value.trim() !== '') { + const parsed = Number(value.trim()); + return Number.isInteger(parsed) && parsed >= 0 ? parsed : null; + } + return null; +} + +/** + * Read the Retry-After delay (in seconds) off any error shape our fetch + * layers produce: a direct `retryAfter` field, or a `retry-after` entry in + * `response.headers` (APIClient stores lowercased headers there). Returns + * null when absent or not a plain second count (the HTTP-date form is + * ignored — callers only ever mention seconds). + */ +export function getErrorRetryAfter(error: unknown): number | null { + if (!error || typeof error !== 'object') return null; + const direct = toRetryAfterSeconds((error as { retryAfter?: unknown }).retryAfter); + if (direct !== null) return direct; + const response = (error as { response?: unknown }).response; + if (response && typeof response === 'object') { + const headers = (response as { headers?: unknown }).headers; + if (headers && typeof headers === 'object') { + if (typeof (headers as { get?: unknown }).get === 'function') { + return toRetryAfterSeconds( + (headers as { get: (name: string) => string | null }).get('retry-after'), + ); + } + const map = headers as Record; + return toRetryAfterSeconds(map['retry-after'] ?? map['Retry-After']); + } + } + return null; +} + /** True when the error is an authentication or authorization failure. */ export function isAuthError(error: unknown): boolean { const status = getErrorStatus(error); From 09fc742da9f57fc67f661dfe675baf6f398afe22 Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Mon, 26 Oct 2026 02:29:32 -0700 Subject: [PATCH 4/5] fix(frontend): forward x-request-id on JSON 4xx and Retry-After through proxies Signed-off-by: Christopher Sellers <1@christophersellers.com> --- .../__tests__/api/proxy-sanitization.test.ts | 78 +++++++++++++++++++ frontend/src/lib/proxy-factory.ts | 8 +- 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/frontend/src/__tests__/api/proxy-sanitization.test.ts b/frontend/src/__tests__/api/proxy-sanitization.test.ts index fd95a265f..a58881601 100644 --- a/frontend/src/__tests__/api/proxy-sanitization.test.ts +++ b/frontend/src/__tests__/api/proxy-sanitization.test.ts @@ -491,6 +491,84 @@ describe('proxy x-request-id passthrough', () => { expect(errorSpy).toHaveBeenCalled(); }); + it.each([400, 422])('preserves the upstream x-request-id on rebuilt JSON proxy %i responses', async (status) => { + const requestId = crypto.randomUUID(); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(JSON.stringify({ + detail: 'invalid payload', + }), { + status, + headers: { 'content-type': 'application/json', 'x-request-id': requestId }, + }))); + const handlers = createJsonProxy({ + serviceName: 'compliance', + buildTargetUrl: () => 'http://internal.service.local/compliance/check', + buildHeaders: () => new Headers(), + methods: ['POST'], + }); + + const response = await handlers.POST!( + makeRouteRequest({ jsonBody: { bad: true } }), + { params: Promise.resolve({ path: ['check'] }) }, + ); + + expect(response.status).toBe(status); + expect(response.headers.get('x-request-id')).toBe(requestId); + const body = await responseJson(response); + expect(body).toEqual({ error: 'invalid payload' }); + }); + + it('passes the upstream Retry-After through on stream proxy 429 responses', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(JSON.stringify({ + detail: 'rate limited', + }), { + status: 429, + headers: { 'content-type': 'application/json', 'retry-after': '45' }, + }))); + const handlers = createStreamProxy({ + serviceName: 'ingestion', + resolveTargetBases: () => ['http://internal.service.local'], + buildHeaders: () => new Headers(), + methods: ['GET'], + }); + + const response = await handlers.GET!( + makeRouteRequest(), + { params: Promise.resolve({ path: ['api/v1/records'] }) }, + ); + + expect(response.status).toBe(429); + expect(response.headers.get('retry-after')).toBe('45'); + }); + + it('preserves Retry-After and x-request-id on rebuilt JSON proxy 429 responses', async () => { + const requestId = crypto.randomUUID(); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(JSON.stringify({ + detail: 'rate limited', + }), { + status: 429, + headers: { + 'content-type': 'application/json', + 'retry-after': '30', + 'x-request-id': requestId, + }, + }))); + const handlers = createJsonProxy({ + serviceName: 'compliance', + buildTargetUrl: () => 'http://internal.service.local/compliance/check', + buildHeaders: () => new Headers(), + methods: ['GET'], + }); + + const response = await handlers.GET!( + makeRouteRequest(), + { params: Promise.resolve({ path: ['check'] }) }, + ); + + expect(response.status).toBe(429); + expect(response.headers.get('retry-after')).toBe('30'); + expect(response.headers.get('x-request-id')).toBe(requestId); + }); + it('sets x-request-id on proxyError responses only when a request ID is provided', () => { const requestId = crypto.randomUUID(); diff --git a/frontend/src/lib/proxy-factory.ts b/frontend/src/lib/proxy-factory.ts index ce5ec9eb5..2cbfa7973 100644 --- a/frontend/src/lib/proxy-factory.ts +++ b/frontend/src/lib/proxy-factory.ts @@ -187,9 +187,14 @@ export function createJsonProxy(config: JsonProxyConfig): MethodHandlers { requestId: response.headers.get('x-request-id') ?? undefined, }); } + const clientHeaders = new Headers(); + const requestId = response.headers.get('x-request-id'); + if (requestId) clientHeaders.set('x-request-id', requestId); + const retryAfter = response.headers.get('retry-after'); + if (retryAfter) clientHeaders.set('retry-after', retryAfter); return NextResponse.json( { error: detail ?? `${config.serviceName} request failed` }, - { status: response.status }, + { status: response.status, headers: clientHeaders }, ); } @@ -249,6 +254,7 @@ const DEFAULT_STREAM_RESPONSE_HEADERS = [ 'content-type', 'content-disposition', 'cache-control', + 'retry-after', 'x-fda-record-count', 'x-request-id', ]; From 4067faae15e62a7e3804fdb217680f4e8defd5ec Mon Sep 17 00:00:00 2001 From: Christopher Sellers <1@christophersellers.com> Date: Mon, 26 Oct 2026 04:01:46 -0700 Subject: [PATCH 5/5] test(frontend): use runtime-generated values in privileged-session-routes tests Replace hardcoded review_id, doc_hash, confidence_score, and created/updated timestamps with crypto.randomUUID(), Math.random(), and Date.now()-derived values, asserting against the same variables. Signed-off-by: Christopher Sellers <1@christophersellers.com> --- .../api/privileged-session-routes.test.ts | 78 +++++++++++-------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/frontend/src/__tests__/api/privileged-session-routes.test.ts b/frontend/src/__tests__/api/privileged-session-routes.test.ts index 75527db7b..40e9267df 100644 --- a/frontend/src/__tests__/api/privileged-session-routes.test.ts +++ b/frontend/src/__tests__/api/privileged-session-routes.test.ts @@ -130,13 +130,18 @@ describe('privileged frontend session forwarding', () => { it('copies the upstream x-request-id onto transformed review queue success responses', async () => { const requestId = crypto.randomUUID(); + const reviewId = crypto.randomUUID(); + const docHash = crypto.randomUUID().replace(/-/g, ''); + const confidenceScore = Math.random(); + const createdAt = new Date(Date.now() - 24 * 60 * 60 * 1000).toISOString(); + const updatedAt = new Date(Date.now()).toISOString(); fetchMock.mockResolvedValueOnce(new Response(JSON.stringify({ items: [{ - review_id: '6ba7b810-9dad-41d1-a0b4-00c04fd430c8', - doc_hash: 'abc123', - confidence_score: 0.42, - created_at: '2026-10-01T00:00:00Z', - updated_at: '2026-10-02T00:00:00Z', + review_id: reviewId, + doc_hash: docHash, + confidence_score: confidenceScore, + created_at: createdAt, + updated_at: updatedAt, status: 'PENDING', tenant_id: 'tenant-123', text_preview: 'preview text', @@ -154,11 +159,11 @@ describe('privileged frontend session forwarding', () => { expect(response.status).toBe(200); expect(response.headers.get('x-request-id')).toBe(requestId); expect(await response.json()).toEqual([{ - id: '6ba7b810-9dad-41d1-a0b4-00c04fd430c8', - doc_hash: 'abc123', - confidence_score: 0.42, - created_at: '2026-10-01T00:00:00Z', - updated_at: '2026-10-02T00:00:00Z', + id: reviewId, + doc_hash: docHash, + confidence_score: confidenceScore, + created_at: createdAt, + updated_at: updatedAt, status: 'PENDING', tenant_id: 'tenant-123', source_text: 'preview text', @@ -167,9 +172,10 @@ describe('privileged frontend session forwarding', () => { }); it('sends no reviewer identity or retired credential on a decision', async () => { + const reviewId = crypto.randomUUID(); fetchMock.mockResolvedValueOnce(new Response(JSON.stringify({ status: 'approved', - review_id: '6ba7b810-9dad-41d1-a0b4-00c04fd430c8', + review_id: reviewId, updated: true, }), { status: 200, @@ -177,8 +183,8 @@ describe('privileged frontend session forwarding', () => { })); const response = await approveReviewItem( - browserRequest('https://app.regengine.test/api/review/6ba7b810-9dad-41d1-a0b4-00c04fd430c8/approve', 'POST'), - { params: Promise.resolve({ id: '6ba7b810-9dad-41d1-a0b4-00c04fd430c8' }) }, + browserRequest(`https://app.regengine.test/api/review/${reviewId}/approve`, 'POST'), + { params: Promise.resolve({ id: reviewId }) }, ); expect(response.status).toBe(200); @@ -253,23 +259,26 @@ describe('privileged frontend session forwarding', () => { ), { params: Promise.resolve({ path: ['flagged-extractions'] }) }, )], - ['review reject', async () => rejectReviewItem( - browserRequest( - 'https://app.regengine.test/api/review/6ba7b810-9dad-41d1-a0b4-00c04fd430c8/reject', - 'POST', - { - extraHeaders: { - 'x-vercel-forwarded-for': '2001:0db8:0:0:0:0:0:1', - 'x-forwarded-for': '192.0.2.10', - 'x-real-ip': '192.0.2.11', - 'x-regengine-proxy-client-ip': '192.0.2.12', - 'x-regengine-proxy-timestamp': '1', - 'x-regengine-proxy-signature': 'v1=attacker', + ['review reject', (() => { + const reviewId = crypto.randomUUID(); + return async () => rejectReviewItem( + browserRequest( + `https://app.regengine.test/api/review/${reviewId}/reject`, + 'POST', + { + extraHeaders: { + 'x-vercel-forwarded-for': '2001:0db8:0:0:0:0:0:1', + 'x-forwarded-for': '192.0.2.10', + 'x-real-ip': '192.0.2.11', + 'x-regengine-proxy-client-ip': '192.0.2.12', + 'x-regengine-proxy-timestamp': '1', + 'x-regengine-proxy-signature': 'v1=attacker', + }, }, - }, - ), - { params: Promise.resolve({ id: '6ba7b810-9dad-41d1-a0b4-00c04fd430c8' }) }, - )], + ), + { params: Promise.resolve({ id: reviewId }) }, + ); + })()], ])('attests the platform client IP on the %s privileged proxy', async (_label, invoke) => { vi.stubEnv('VERCEL', '1'); vi.stubEnv('VERCEL_ENV', 'production'); @@ -306,10 +315,13 @@ describe('privileged frontend session forwarding', () => { browserRequest('https://app.regengine.test/api/review/flagged-extractions'), { params: Promise.resolve({ path: ['flagged-extractions'] }) }, )], - ['review reject', async () => rejectReviewItem( - browserRequest('https://app.regengine.test/api/review/6ba7b810-9dad-41d1-a0b4-00c04fd430c8/reject', 'POST'), - { params: Promise.resolve({ id: '6ba7b810-9dad-41d1-a0b4-00c04fd430c8' }) }, - )], + ['review reject', (() => { + const reviewId = crypto.randomUUID(); + return async () => rejectReviewItem( + browserRequest(`https://app.regengine.test/api/review/${reviewId}/reject`, 'POST'), + { params: Promise.resolve({ id: reviewId }) }, + ); + })()], ])('fails the %s privileged proxy closed when Vercel omits the platform IP', async (_label, invoke) => { vi.stubEnv('VERCEL', '1'); vi.stubEnv('VERCEL_ENV', 'production');