feat: improve frontend error visibility - #151
Conversation
Users had no way to tell "no data" from "something failed" — a failed
query rendered the exact same empty state as a genuinely empty result,
and a failed mutation (status change, note/contact/interview-round
CRUD, logout) just did nothing with zero feedback. No error boundary
existed either, so an uncaught render exception blanked the page.
Backend (small fixes everything else leans on):
- NotFoundError (AppError.ts) was double-appending " not found" when
the use-case's message already ended with it ("Application not
found" -> "Application not found not found" on the wire) — fixed,
only appends when absent.
- 3 use-cases (GenerateCoverLetterUseCase, ComputeHealthScoreUseCase,
ParseJobDescriptionUseCase) threw plain uncoded Errors for their
not-found/forbidden/validation cases, so formatError.ts masked them
as generic "Internal server error" instead of the real reason.
Attached the missing ERROR_CODES, matching the pattern every sibling
use-case already follows.
Frontend:
- New src/lib/errors.ts: getErrorMessage(), the one place that maps
any thrown error (GraphQL, network, or otherwise) to a user-facing
string, keyed off extensions.code. Replaces three different ad-hoc
extractors (login.tsx's local extractGqlError, two bare hardcoded
fallback strings in the application forms).
- Added sonner + a global MutationCache.onError in queryClient.ts —
fixes every mutation that only wired onSuccess (board drag-and-drop
status change, bulk actions, note/contact/interview-round CRUD,
logout) with one change, since it fires in addition to any
mutation-level handler.
- Added an errorComponent to the root route so a render exception
shows a fallback with a reload button instead of a blank page.
- New ErrorState component (mirrors the app's existing empty-state
visual convention, in red, with a retry action) wired into every
primary query's isLoading/isError/empty ternary chain: dashboard,
applications board, applications list, application detail (+ its
notes query), analytics.
Verified end-to-end against a live dev server: surgically failing just
the ApplicationsPage query (not the session refresh, which this app's
plain-<a> sidebar nav depends on for every navigation) shows ErrorState
with working retry; failing a mutation shows a sonner toast; both
render correctly in dark mode.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N2PBmsuzPhrmNnfZf6C3BM
WalkthroughThe API now emits structured error codes and avoids duplicate not-found suffixes. The web app centralises error messages, adds mutation toasts, route fallbacks, and retryable query states, and updates authentication and application forms to use the shared handling. ChangesError handling flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LoginPage
participant GraphQLAPI
participant getErrorMessage
LoginPage->>GraphQLAPI: submit credentials or verification code
GraphQLAPI-->>LoginPage: return success or coded error
LoginPage->>getErrorMessage: resolve caught error
getErrorMessage-->>LoginPage: return display message
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview deployments for this PR: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/lib/errors.ts`:
- Around line 13-15: Update isGraphQLErrorLike to require that error.response is
a non-null object before treating the value as GraphQLErrorLike, so responses
such as null are rejected and the generic fallback remains available.
In `@apps/web/src/lib/queryClient.ts`:
- Around line 5-13: The comment describing mutationCache.onError incorrectly
implies locally handled mutations are excluded. Update the surrounding comment
for queryClient and MutationCache to state that the global handler runs for
every mutation failure, including mutations with their own onError, unless an
explicit opt-out mechanism is implemented; do not claim inline-handled mutations
are skipped.
In `@apps/web/src/routes/_authenticated/dashboard.tsx`:
- Around line 118-119: Update the dashboard rendering around the Recent
applications error branch and metric cards so a failed query does not calculate
or display zero-valued metrics from empty fallback data. Render a page-level
ErrorState for the query failure, or propagate the error/loading state to the
stat cards while preserving normal metrics for successful queries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b8a19ebf-93f5-4203-98a4-3140a6bc3b41
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
apps/api/src/__tests__/application/coverLetter/GenerateCoverLetterUseCase.test.tsapps/api/src/__tests__/application/healthScore/ComputeHealthScoreUseCase.test.tsapps/api/src/__tests__/application/jobDescription/ParseJobDescriptionUseCase.test.tsapps/api/src/__tests__/http/errors/AppError.test.tsapps/api/src/__tests__/http/errors/formatError.test.tsapps/api/src/http/errors/AppError.tsapps/api/src/use-cases/application/ComputeHealthScoreUseCase.tsapps/api/src/use-cases/coverLetter/GenerateCoverLetterUseCase.tsapps/api/src/use-cases/jobDescription/ParseJobDescriptionUseCase.tsapps/web/package.jsonapps/web/src/__tests__/components/EditApplicationPage.test.tsxapps/web/src/__tests__/components/LoginPage.test.tsxapps/web/src/__tests__/components/NewApplicationPage.test.tsxapps/web/src/components/ErrorState.tsxapps/web/src/constants.tsapps/web/src/lib/errors.tsapps/web/src/lib/queryClient.tsapps/web/src/routes/__root.tsxapps/web/src/routes/_authenticated/-analytics-page.tsxapps/web/src/routes/_authenticated/applications/$applicationId/edit.tsxapps/web/src/routes/_authenticated/applications/$applicationId/index.tsxapps/web/src/routes/_authenticated/applications/-board-page.tsxapps/web/src/routes/_authenticated/applications/index.tsxapps/web/src/routes/_authenticated/applications/new.tsxapps/web/src/routes/_authenticated/dashboard.tsxapps/web/src/routes/login.tsx
| function isGraphQLErrorLike(error: unknown): error is GraphQLErrorLike { | ||
| return typeof error === 'object' && error !== null && 'response' in error; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Validate the response value, not only its presence.
{ response: null } passes this guard, then Line 42 throws while reading response.errors; the recovery UI fails instead of showing the generic fallback.
Proposed fix
function isGraphQLErrorLike(error: unknown): error is GraphQLErrorLike {
- return typeof error === 'object' && error !== null && 'response' in error;
+ return (
+ typeof error === 'object' &&
+ error !== null &&
+ 'response' in error &&
+ typeof (error as { response?: unknown }).response === 'object' &&
+ (error as { response?: unknown }).response !== null
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function isGraphQLErrorLike(error: unknown): error is GraphQLErrorLike { | |
| return typeof error === 'object' && error !== null && 'response' in error; | |
| } | |
| function isGraphQLErrorLike(error: unknown): error is GraphQLErrorLike { | |
| return ( | |
| typeof error === 'object' && | |
| error !== null && | |
| 'response' in error && | |
| typeof (error as { response?: unknown }).response === 'object' && | |
| (error as { response?: unknown }).response !== null | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/lib/errors.ts` around lines 13 - 15, Update isGraphQLErrorLike
to require that error.response is a non-null object before treating the value as
GraphQLErrorLike, so responses such as null are rejected and the generic
fallback remains available.
| export const queryClient = new QueryClient({ | ||
| // Fires for every mutation that doesn't already handle its own error (in | ||
| // addition to, not instead of, any mutation-level onError) — this is the | ||
| // safety net for the many mutations (board drag-and-drop, bulk actions, | ||
| // note/contact/interview-round CRUD, logout) that previously failed with | ||
| // no feedback to the user at all. | ||
| mutationCache: new MutationCache({ | ||
| onError: (error) => toast.error(getErrorMessage(error)), | ||
| }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate queryClient file =="
fd -a 'queryClient\.ts$' . || true
echo "== relevant file =="
if [ -f apps/web/src/lib/queryClient.ts ]; then
cat -n apps/web/src/lib/queryClient.ts
fi
echo "== package versions =="
for f in package.json apps/web/package.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
node -e "const p=require('./$f'); console.log(JSON.stringify({dependencies:p.dependencies?.['`@tanstack/react-query`'], devDependencies:p.devDependencies?.['`@tanstack/react-query`']}, null, 2))"
fi
done
echo "== usages of toast/error handling in web =="
rg -n "toast\.(error|success)|onError:|onSuccess:|onSettled:" apps/web/src --glob '*.{ts,tsx}' | head -200Repository: mankatcheung/job-finder
Length of output: 2964
🌐 Web query:
TanStack Query 5 MutationCache onError mutation-level onError callbacks invoked together documentation
💡 Result:
In TanStack Query v5, when a mutation encounters an error, the onError callbacks defined in the MutationCache and those defined at the mutation level (via useMutation or the mutate function) are both invoked [1][2]. They are not mutually exclusive, and the MutationCache callback does not "override" or prevent the mutation-level callbacks from running [3][2][4]. Execution Order The callbacks are executed in a specific order within the mutation lifecycle [1]: 1. MutationCache onError: The global onError handler defined in the MutationCache is executed first [1][2]. 2. Mutation-level onError: The onError handler defined in the useMutation hook or the mutate function is executed second [1]. Key Behavioral Notes - Global vs. Local: The MutationCache onError handler is designed to be a global, always-called handler (e.g., for global error logging or toast notifications), while mutation-level handlers are intended for component-specific logic [3][5][2]. - No Suppression: There is no built-in mechanism to "stop" the chain or suppress the global MutationCache handler when a local handler is present [2]. If you need to avoid triggering the global handler conditionally, common workarounds include checking meta information or variables within the MutationCache handler to decide whether to execute its logic [6][7]. - Lifecycle Difference: Mutation-level callbacks (especially those passed to mutate) are subject to component lifecycle constraints—they may not execute if the component unmounts before the mutation finishes [5][8][4]. In contrast, MutationCache handlers are not tied to component lifecycles and will always execute for any mutation error [3][2].
Citations:
- 1: https://cdn.jsdelivr.net/npm/@tanstack/query-core@5.101.4/src/mutation.ts
- 2: Conditionally re-throw errors on onError and globally handle uncaught errors TanStack/query#3632
- 3: https://tanstack.com/query/v5/docs/reference/MutationCache
- 4: local onError is not working as expected when using global onError on MutationCache TanStack/query#1927
- 5: https://tanstack.com/query/v5/docs/framework/react/guides/mutations
- 6: Is there a way to provide fallback `onError` handler, which doesn't get called when `onError` option is passed to `mutate` call? TanStack/query#3013
- 7: Possibility of accessing mutation's observers (MutationCache onError) TanStack/query#6671
- 8: https://tanstack.com/query/v5/docs/framework/react/guides/mutations.md
Do not imply that locally handled mutations are excluded.
MutationCache.onError is global in TanStack Query v5 and still runs when a mutation also defines its own onError; this handler has no opt-out marker. Mutations with inline error handling may therefore display this toast in addition to their local feedback, so either document that all mutation failures are toasted or add an explicit guard.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/lib/queryClient.ts` around lines 5 - 13, The comment describing
mutationCache.onError incorrectly implies locally handled mutations are
excluded. Update the surrounding comment for queryClient and MutationCache to
state that the global handler runs for every mutation failure, including
mutations with their own onError, unless an explicit opt-out mechanism is
implemented; do not claim inline-handled mutations are skipped.
| ) : isError ? ( | ||
| <ErrorState error={error} onRetry={() => refetch()} /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Avoid showing zero-valued metrics after a failed query.
Line 118 renders an error only for Recent applications, while the stat cards above receive loading={false} and calculate from an empty fallback. A failed request therefore displays misleading zero counts. Render a page-level error state, or keep the metric cards in an error/loading state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/routes/_authenticated/dashboard.tsx` around lines 118 - 119,
Update the dashboard rendering around the Recent applications error branch and
metric cards so a failed query does not calculate or display zero-valued metrics
from empty fallback data. Render a page-level ErrorState for the query failure,
or propagate the error/loading state to the stat cards while preserving normal
metrics for successful queries.
Resolves conflicts with the frontend-error-visibility work (#151), which touched the same NOT_FOUND/FORBIDDEN/VALIDATION coded-error sites in GenerateCoverLetterUseCase/ParseJobDescriptionUseCase and added getErrorMessage/ErrorState to the application detail page. Kept both: the AI_NOT_CONFIGURED provider-factory check from this branch alongside #151's error coding/messaging fixes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N2PBmsuzPhrmNnfZf6C3BM
1 participant
Summary
Users had no way to tell "no data" from "something failed": a failed query rendered the exact same empty state as a genuinely empty result, and a failed mutation (board drag-and-drop status change, bulk actions, note/contact/interview-round CRUD, logout) just did nothing with zero feedback. No error boundary existed either, so an uncaught render exception would blank the page.
Backend (small, fixed first since the frontend leans on reliable error codes)
NotFoundError(AppError.ts) was double-appending" not found"when the use-case's message already ended with it —"Application not found"became"Application not found not found"on the wire. Fixed to only append when absent.GenerateCoverLetterUseCase,ComputeHealthScoreUseCase,ParseJobDescriptionUseCase) threw plain uncodedErrors for their not-found/forbidden/validation cases, soformatError.tsmasked them as a generic"Internal server error"instead of the real reason. Attached the missingERROR_CODES, matching the pattern every sibling use-case already follows.Frontend
src/lib/errors.ts:getErrorMessage()— the one place that maps any thrown error (GraphQL, network, or otherwise) to a user-facing string, keyed offextensions.code. Replaces three different ad-hoc extractors that existed before this (login's localextractGqlError, two bare hardcoded fallback strings in the application forms).sonner+ a globalMutationCache.onErrorinqueryClient.ts— fixes every mutation that only wiredonSuccesswith a single change, since it fires in addition to any mutation-level handler that already exists (e.g. the login/register forms keep their own inline error box unchanged).errorComponentto the root route so a render exception shows a friendly fallback with a reload button instead of a blank page.ErrorStatecomponent (mirrors the app's existing empty-state visual convention — icon, centered text,py-12— just in red, with a retry action) wired into every primary query'sisLoading/isError/empty ternary chain: dashboard, applications board, applications list, application detail (+ its notes query), analytics.Test plan
pnpm --filter @job-finder/api typecheck/test(746 tests) /build— all cleanpnpm --filter @job-finder/web typecheck/lint/build— all cleanpnpm --filter @job-finder/web test— at the pre-existing baseline (39 failing, unrelatedlocalStorage-in-jsdom environment issue confirmed via git-stash comparison, not introduced by this PR); zero new regressionsApplicationsPagequery (not the session refresh, which this app's plain-<a>sidebar nav depends on for every navigation) — confirmedErrorStaterenders with a working "Try again" that recovers once the query succeeds again; failed aCreateNotemutation — confirmed asonnertoast renders with a readable message; confirmed both render correctly in dark mode🤖 Generated with Claude Code
https://claude.ai/code/session_01N2PBmsuzPhrmNnfZf6C3BM
Summary by CodeRabbit
New Features
Bug Fixes