feat(frontend): Error classification & SectionError component (#791)#814
feat(frontend): Error classification & SectionError component (#791)#814ericsocrat merged 8 commits intomainfrom
Conversation
- Add classifyError() utility (network/auth/server/unknown categories) - Enhance ErrorBoundary PageFallback with per-category illustrations and i18n - Auth errors show 'Sign in' link; network errors show 'offline' illustration - Create SectionError standalone component for TanStack Query error states - Add i18n keys for all error categories in en/pl/de dictionaries - Tests: error-classifier (19), SectionError (9), ErrorBoundary classification (6)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bundle Size Report
✅ Bundle size is within acceptable limits. |
There was a problem hiding this comment.
Pull request overview
Adds user-friendly error categorization to the frontend so error UIs (page-level and section-level) can show clearer, localized messaging and recovery actions.
Changes:
- Introduces
classifyError()utility to map errors intonetwork | auth | server | unknown. - Enhances
ErrorBoundarywith category-specific i18n copy, illustrations, and an auth-specific “Sign in” CTA. - Adds
SectionErrorinline component for TanStack Query-style error states, plus new tests and i18n keys (en/pl/de).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/lib/error-classifier.ts | New error classification helper used by error UIs. |
| frontend/src/lib/error-classifier.test.ts | Unit tests for classifyError() categorization + priority rules. |
| frontend/src/components/common/index.ts | Re-exports SectionError from common components barrel. |
| frontend/src/components/common/SectionError.tsx | New inline error card component with retry + i18n. |
| frontend/src/components/common/SectionError.test.tsx | Tests for rendering, category behavior, label, and retry. |
| frontend/src/components/common/ErrorBoundary.tsx | Category-aware page fallback UI + auth-specific CTA. |
| frontend/src/components/common/ErrorBoundary.test.tsx | Adds tests verifying category-specific rendering and links. |
| frontend/messages/en.json | Adds new errorBoundary.* and sectionError.* keys. |
| frontend/messages/pl.json | Adds Polish translations for new error keys. |
| frontend/messages/de.json | Adds German translations for new error keys. |
| CHANGELOG.md | Documents newly added error classification + UI components. |
| <button | ||
| onClick={onRetry} | ||
| className={buttonClasses("primary", "sm")} | ||
| data-testid="section-error-retry" | ||
| > |
There was a problem hiding this comment.
This retry control is a plain without an explicit type. When SectionError is rendered inside a (e.g., within a settings page section), the default type="submit" can cause accidental form submission. Add type="button" to make the intent explicit.
| <a | ||
| href="/auth/login" | ||
| className={buttonClasses("secondary", "md")} | ||
| > | ||
| {t("errorBoundary.signIn")} | ||
| </a> |
There was a problem hiding this comment.
These are internal navigation actions styled as buttons, but they use raw tags. In this codebase, the Button module provides (Next.js ) specifically for navigation actions that should look like buttons; using it preserves client-side navigation/prefetch and keeps behavior consistent. Consider switching both this auth link and the /app link to ButtonLink (or at least next/link).
| <button | ||
| onClick={onReset} | ||
| className={buttonClasses("primary", "md")} | ||
| > | ||
| {t("common.tryAgain")} | ||
| </button> |
There was a problem hiding this comment.
This "Try again" control is a plain without an explicit type. If this boundary ever renders inside a , the default type="submit" can trigger unintended submissions. Set type="button" for non-submit buttons here (and in similar fallbacks) to avoid that behavior.
…ries # Conflicts: # CHANGELOG.md
| <button | ||
| onClick={onRetry} | ||
| className={buttonClasses("primary", "sm")} | ||
| data-testid="section-error-retry" | ||
| > | ||
| <RefreshCw size={14} className="mr-1 inline" aria-hidden="true" /> | ||
| {t("common.tryAgain")} | ||
| </button> |
There was a problem hiding this comment.
The retry <button> defaults to type="submit", which can accidentally submit a surrounding form if SectionError is rendered inside one. Add type="button" to make the retry action non-submitting by default (consistent with other common components like Alert/Toggle/PrintButton).
| export function classifyError(error: Error): ErrorCategory { | ||
| const msg = error.message.toLowerCase(); | ||
| const name = error.name.toLowerCase(); | ||
| const combined = `${name} ${msg}`; | ||
|
|
There was a problem hiding this comment.
Given this utility is intended to classify errors coming from various sources (including query libraries where the error is frequently typed as unknown), accepting only Error forces callers to cast and can be awkward at call sites. Consider changing the signature to accept unknown and normalizing to a message/name internally (e.g., handle non-Error thrown values) so both ErrorBoundary and SectionError can pass through query errors without unsafe casts.
| export function classifyError(error: Error): ErrorCategory { | |
| const msg = error.message.toLowerCase(); | |
| const name = error.name.toLowerCase(); | |
| const combined = `${name} ${msg}`; | |
| export function classifyError(error: unknown): ErrorCategory { | |
| let rawMessage = ""; | |
| let rawName = ""; | |
| if (error instanceof Error) { | |
| rawMessage = error.message || ""; | |
| rawName = error.name || ""; | |
| } else if (typeof error === "string") { | |
| rawMessage = error; | |
| } else if (error && typeof error === "object") { | |
| const maybeMessage = (error as { message?: unknown }).message; | |
| const maybeName = (error as { name?: unknown }).name; | |
| if (typeof maybeMessage === "string") { | |
| rawMessage = maybeMessage; | |
| } | |
| if (typeof maybeName === "string") { | |
| rawName = maybeName; | |
| } | |
| } else if (error != null) { | |
| rawMessage = String(error); | |
| } | |
| const msg = rawMessage.toLowerCase(); | |
| const name = rawName.toLowerCase(); | |
| const combined = `${name} ${msg}`.trim(); |
| const AUTH_PATTERNS = [ | ||
| "jwt expired", | ||
| "jwt", | ||
| "not authenticated", | ||
| "unauthorized", | ||
| "403", | ||
| "401", | ||
| "auth", | ||
| "permission denied", | ||
| "access denied", | ||
| "invalid login", | ||
| "session expired", | ||
| "refresh_token", | ||
| "pgrst301", | ||
| ] as const; |
There was a problem hiding this comment.
Using substring matching for status codes (e.g., \"401\", \"403\") can misclassify unrelated errors that happen to contain those digits as part of a larger number/token (e.g., \"1401\", \"E4012\"). To reduce false positives, prefer regex-based matching with boundaries for HTTP codes (and keep string patterns for phrases), or split auth detection into more specific checks.
| if (NETWORK_PATTERNS.some((p) => combined.includes(p))) { | ||
| return "network"; | ||
| } | ||
|
|
||
| // Auth/permission errors | ||
| if (AUTH_PATTERNS.some((p) => combined.includes(p))) { | ||
| return "auth"; | ||
| } |
There was a problem hiding this comment.
Using substring matching for status codes (e.g., \"401\", \"403\") can misclassify unrelated errors that happen to contain those digits as part of a larger number/token (e.g., \"1401\", \"E4012\"). To reduce false positives, prefer regex-based matching with boundaries for HTTP codes (and keep string patterns for phrases), or split auth detection into more specific checks.
Closes #791. Add classifyError() utility (network/auth/server/unknown), enhance ErrorBoundary with per-category illustrations and i18n, create SectionError standalone component, add 34 new tests, add i18n keys for en/pl/de.