Repository navigation
feat(web): map API error codes to safe user-facing messages (mapApiError utility) - #143
Conversation
Closes #124 Add src/lib/errors.ts: a pure mapApiError(err, opts) that normalizes an AxiosError / Error / unknown into { code, status, userMessage, retryable, raw } using a frozen ERROR_MESSAGES table keyed by backend error code. The UI never interpolates a raw server message into userMessage (default off); the original error travels in raw for telemetry only. Adds 12 unit tests covering code extraction order, the message table, status-derived codes, retryable mapping, and a regression that raw server text is not surfaced when showRaw is false. Workstream 1 (the utility) only; adoption across pages/hooks is web-err-05.
EmeditWeb
left a comment
There was a problem hiding this comment.
⚠️ Automated Audit: partial
@EmeditWeb Good start — please look into the gaps identified below.
The diff implements exactly Workstream 1 of #124: a new pure src/lib/errors.ts exporting the ApiError type, a frozen ERROR_MESSAGES table containing every code the issue specifies plus DEFAULT, and mapApiError with the required extraction order (data.errorCode → data.code → HTTP_<status>/NETWORK → UNKNOWN), retryable=true only for NETWORK/5xx, and a dev-only VITE_SHOW_RAW_ERRORS build-env switch that defaults off so raw server text is never interpolated into userMessage. All four acceptance criteria are covered by the 12 accompanying unit tests, including the explicit regression test that a leaking Postgres constraint message is not surfaced when showRaw is false, plus unknown-code fallback, null/undefined/string input, and raw-preserved-for-telemetry cases. Scope discipline holds: no new deps, no page/hook adoption (correctly fenced to web-err-05), no workflow/test-config changes, no secrets, no hardcoded endpoints. Note on verification: the build-and-lint CI check was still PENDING at audit time and the sandbox run was skipped, so the body's claimed lint/build/coverage numbers are not yet independently confirmed — this is a tooling-timing fact, not an author gap, but the claims should not be credited until CI reports green. The bot's 'vague title' flag appears to be a keyword-overlap false positive: the title names the utility (mapApiError), the area (web), uses a conventional-commit prefix, and links Closes #124.
⚖️ Adjusted by bot policy: confidence 78% is below the 90% threshold for a full approval; the PR title is vague — it must state what was fixed and where.
CI checks: ⏳ PENDING: build-and-lint
Merge conflicts: ✅ none
Audited by stepfi-audit-bot 🤖
EmeditWeb
left a comment
There was a problem hiding this comment.
⚠️ Automated Audit: partial
@EmeditWeb Good start — please look into the gaps identified below.
The diff implements exactly what the issue's Workstream 1 specifies: a new pure src/lib/errors.ts with the ApiError type, a frozen ERROR_MESSAGES table containing all six specified codes plus DEFAULT, code extraction in the required order (response.data.errorCode → response.data.code → HTTP_<status>/NETWORK → UNKNOWN), retryable true for NETWORK/5xx and false for 4xx, and an env-gated VITE_SHOW_RAW_ERRORS that defaults off so raw server text never reaches userMessage. All four acceptance criteria are covered by the 12 added unit tests, including the explicit regression test that a leaky SQL-constraint server message is not surfaced when showRaw is false, and the unknown-code/null/undefined/string inputs are proven not to throw. Adoption across Sponsors/Vouch/useTransaction is explicitly deferred to web-err-05 by the issue's own scope fence, and the src/services/api.ts normalization was marked optional, so neither omission is a gap. Independent verification is green (build-and-lint passed, no merge conflicts, CI config untouched); the body's specific coverage percentages are not independently reproduced here (the base repo has no coverage config), so those exact numbers should be read as unverified claims even though the overall checks passed. I treat the bot's "vague title" flag as a false positive: the title is a conventional-commit line naming the exact utility added and the issue it closes, and it does share the issue's key terms (error, message, mapping, utility).
⚖️ Adjusted by bot policy: confidence 84% is below the 90% threshold for a full approval; the PR title is vague — it must state what was fixed and where.
CI checks: ✅ PASSED: build-and-lint
Merge conflicts: ✅ none
Audited by stepfi-audit-bot 🤖
EmeditWeb
left a comment
There was a problem hiding this comment.
⚠️ Automated Audit: partial
@EmeditWeb Good start — please look into the gaps identified below.
The PR adds exactly the scoped Workstream 1 deliverable from #124: a pure src/lib/errors.ts exporting mapApiError, the frozen ERROR_MESSAGES table, and the ApiError type, with code extraction order (errorCode → code → HTTP_<status> → NETWORK/UNKNOWN), retryable mapping (NETWORK/5xx true, 4xx false), and a default-off VITE_SHOW_RAW_ERRORS build-env flag — all matching the issue's Implementation steps. All four Workstream 1 acceptance criteria are backed by tests (409/ROLE_ALREADY_SET → table text + retryable:false; non-axios Error → UNKNOWN/DEFAULT; no-response → NETWORK/retryable:true; and a regression asserting the raw server message is not surfaced when raw display is off), and the diff substantiates the security claim (no raw message reaches userMessage by default). The issue explicitly fences page/hook adoption (web-err-05) and business-logic changes out of scope and marks api.ts normalization as optional, so not wiring call sites is not a gap; structure (Summary/Changes/Verification/Closes #124) is present, CI passed with no config changes, and the deterministic findings show no red flags. The only unverified item is the test:coverage claim (the sandbox run was skipped, so independent tests never ran), which is a tooling fact, not an author-fixable code gap.
⚖️ Adjusted by bot policy: confidence 75% is below the 90% threshold for a full approval.
CI checks: ✅ PASSED: build-and-lint
Merge conflicts: ✅ none
Audited by stepfi-audit-bot 🤖
EmeditWeb
left a comment
There was a problem hiding this comment.
⚠️ Automated Audit: partial
@EmeditWeb Good start — please look into the gaps identified below.
The diff implements exactly what Workstream 1 of #124 specifies: a new pure src/lib/errors.ts exporting the ApiError type, a frozen ERROR_MESSAGES table with all six required entries plus DEFAULT, and mapApiError that extracts the code in the prescribed order (response.data.errorCode → data.code → HTTP_<status>/NETWORK → UNKNOWN), sets retryable true only for NETWORK/5xx, carries the original error in raw without rendering it, and gates raw-message appending behind build-time VITE_SHOW_RAW_ERRORS (default off). The accompanying 12 unit tests cover every stated acceptance criterion, including the regression that a leaky server message (duplicate key ... constraint) never reaches userMessage when showRaw is false, unknown-code fallback without throwing, and null/undefined/string inputs. No new dependencies, no out-of-scope edits, no CI/workflow or test-config changes, and no deterministic red flags; independent CI (build-and-lint) is green with no merge conflicts. Two caveats, neither verdict-lowering: the body's npm run test:coverage percentages and a "60/50/65/60 floor" are not corroborated by the independent run (base branch has no coverage config), so treat those numbers as unverified; and the bot's "vague title" flag is a keyword-overlap false positive — the title concretely states the fix and scope (feat(web): map API error codes to safe user-facing messages (mapApiError utility)).
⚖️ Adjusted by bot policy: confidence 86% is below the 90% threshold for a full approval; the PR title is vague — it must state what was fixed and where.
CI checks: ✅ PASSED: build-and-lint
Merge conflicts: ✅ none
Audited by stepfi-audit-bot 🤖
EmeditWeb
left a comment
There was a problem hiding this comment.
⚠️ Automated Audit: partial
@EmeditWeb Good start — please look into the gaps identified below.
The diff implements exactly what Workstream 1 of #124 specifies: a new pure src/lib/errors.ts with the ApiError type, a frozen ERROR_MESSAGES table containing all the mandated default strings, code extraction in the required precedence (data.errorCode → data.code → HTTP_<status>/NETWORK → UNKNOWN), correct retryable mapping (NETWORK + 5xx true, 4xx false), and a dev-only VITE_SHOW_RAW_ERRORS switch that defaults off so the raw server string is never interpolated into userMessage (it only travels in raw for telemetry). The accompanying 12-test file covers every stated acceptance criterion, including the explicit regression that a SQL-leaking server message does not reach userMessage when showRaw is false, plus unknown-code/null/string tolerance. Independent verification is green (build-and-lint passed, no merge conflicts), CI config untouched, no secrets, no new dependencies, and the optional api.ts normalization was explicitly optional in the issue. Two neutral notes: the body's coverage figures and the claimed "60/50/65/60 floor" are not independently confirmed (the visible CI job is build-and-lint and the repo shows no coverage config), so that number is an uncredited claim; and the bot's "vague title" flag appears to be a keyword-overlap false positive — the title "feat(web): map API error codes to safe user-facing messages (mapApiError utility)" states the essence and area precisely.
⚖️ Adjusted by bot policy: confidence 82% is below the 90% threshold for a full approval; the PR title is vague — it must state what was fixed and where.
CI checks: ✅ PASSED: build-and-lint
Merge conflicts: ✅ none
Audited by stepfi-audit-bot 🤖
EmeditWeb
left a comment
There was a problem hiding this comment.
✅ Automated Audit: solves
@EmeditWeb Excellent work, thank you! 🎉
The diff implements exactly the Workstream 1 root cause: a new pure src/lib/errors.ts exporting a frozen ERROR_MESSAGES table, the ApiError type, and mapApiError that derives the code in the specified order (response.data.errorCode → response.data.code → HTTP_<status> / NETWORK → UNKNOWN), maps retryable true only for NETWORK/5xx, and returns table text so a raw server string is never interpolated into userMessage (raw is kept only on raw for telemetry). All four acceptance criteria are directly covered by the 12 added unit tests, including the explicit regression that a SQL-fragment server message does not leak when showRaw is false, plus unknown-code, status-derived, null/undefined/string, and retryable-mapping cases. SHOW_RAW_ERRORS is read from build-time VITE_SHOW_RAW_ERRORS and defaults off; no new dependencies, no page/service business logic touched (the api.ts normalization and page adoption are explicitly optional/out of scope per the issue). Title and body meet the structure contract (Summary, Changes, Verification with exact commands, Closes #124); independent CI (build-and-lint) is green with no merge conflicts and no CI-config changes — note the CI job name covers build/lint, so the body's coverage percentages are not independently reproduced, but nothing in the independent run contradicts them. Minor nit only: the dev-only raw-message switch relies on the env value alone rather than additionally gating on import.meta.env.DEV, which would harden the 'never true in production' guarantee.
CI checks: ✅ PASSED: build-and-lint
Merge conflicts: ✅ none
Audited by stepfi-audit-bot 🤖
Summary
Closes #124
Adds a single
mapApiErrorutility so every surface shows the same safe, human text for a backend error — translated through one code-keyed table instead of interpolating a raw server string into the UI. Pure and side-effect free; the original error travels inrawfor telemetry and is never rendered to the DOM.What changed
src/lib/errors.ts(new):mapApiError(err, opts) → { code, status, userMessage, retryable, raw }, a frozenERROR_MESSAGEStable, and theApiErrortype. Code is read fromresponse.data.errorCode→response.data.code→ status-derived (NETWORK/HTTP_<status>) →UNKNOWN.retryableis true forNETWORKand 5xx.VITE_SHOW_RAW_ERRORS(default off, dev-only) can append the raw message for debugging.src/lib/__tests__/errors.test.ts(new): 12 unit tests.This is Workstream 1 (the utility) only; adopting it across pages/hooks is tracked as web-err-05 and is intentionally out of scope here.
This repo is for the React web app only
Type of change
Testing
Verification
Ran exactly what CI runs:
npm run lint→ 0 errorsnpm run test:coverage→ all tests pass (added 12; suite coverage 69.15% statements / 63.33% branches / 73.39% functions / 69.62% lines, above the 60/50/65/60 floor).src/lib/errors.ts: 100% statements/functions/lines.npm run build→ tsc -b clean + vite build succeeded.Acceptance criteria from #124 covered by tests: the 409/
ROLE_ALREADY_SETenvelope resolves to the table text +retryable:false; a non-axiosError→UNKNOWN+ DEFAULT; no-response →NETWORK+retryable:true; and a regression asserting the raw server message is never surfaced whenSHOW_RAW_ERRORSis off.Context files reviewed
src/lib/errors.ts+ tests).