Skip to content

feat(web): map API error codes to safe user-facing messages (mapApiError utility) - #143

Merged
EmeditWeb merged 1 commit into
mainfrom
feat/map-api-error
Oct 7, 2026
Merged

EmeditWeb merged 1 commit into
mainfrom
feat/map-api-error

Conversation

@EmeditWeb

Copy link
Copy Markdown
Member

Summary

Closes #124

Adds a single mapApiError utility 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 in raw for telemetry and is never rendered to the DOM.

What changed

  • src/lib/errors.ts (new): mapApiError(err, opts) → { code, status, userMessage, retryable, raw }, a frozen ERROR_MESSAGES table, and the ApiError type. Code is read from response.data.errorCode → response.data.code → status-derived (NETWORK / HTTP_<status>) → UNKNOWN. retryable is true for NETWORK and 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

  • My changes are inside src/
  • I have NOT added Rust, Soroban, or contract code
  • I have NOT added React Native or Expo-specific code
  • I have NOT hardcoded hex color values
  • All icons are from lucide-react only (no icons added)
  • No API calls made directly in page files (use services/ layer only)

Type of change

  • Service layer addition

Testing

  • npm run build passes with zero errors
  • npm run lint passes with zero errors
  • Loading / error / empty states, 375px mobile, browser console: N/A — this is a pure utility with no UI.

Verification

Ran exactly what CI runs:

  • npm run lint → 0 errors
  • npm 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_SET envelope resolves to the table text + retryable:false; a non-axios Error → UNKNOWN + DEFAULT; no-response → NETWORK + retryable:true; and a regression asserting the raw server message is never surfaced when SHOW_RAW_ERRORS is off.

Context files reviewed

  • progress-tracker.md not updated — outside this issue's scope fence (the issue scopes changes to src/lib/errors.ts + tests).

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 EmeditWeb left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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 EmeditWeb left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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 EmeditWeb changed the title feat(web): add mapApiError code→message mapping utility (Closes #124) feat(web): map API error codes to safe user-facing messages (mapApiError utility) Oct 6, 2026

@EmeditWeb EmeditWeb left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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 EmeditWeb left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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 EmeditWeb left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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 EmeditWeb left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 🤖

@EmeditWeb
EmeditWeb merged commit ed9d1ff into main Oct 7, 2026
2 checks passed
@EmeditWeb
EmeditWeb deleted the feat/map-api-error branch October 7, 2026 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(Medium) Error Codes: API error-code to user-message mapping utility

1 participant