Resolve accept invite API errors into readable messages - #45
Conversation
Throw ThunderIDAPIError instead of a raw Error so the API response body is not surfaced to users in the React and Vue accept invite components. Refs thunder-id/thunderid#4373
📝 WalkthroughWalkthroughReact and Vue ChangesInvite-flow error handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/javascript/src/errors/__tests__/ThunderIDAPIError.test.ts (1)
181-195: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMake the regression test prove the new flow behavior.
This test constructs
ThunderIDAPIErrordirectly. It can pass even if either changedAcceptInvitecomponent still throws a rawErroror passes incorrect metadata. Add component-level tests for the React and Vue non-OK/flow/executepaths. Assert the error type, origin, stable flow code,statusCode,statusText, and parsed message.Also assert the exact parsed message here. The current negative assertions would pass for an empty or unrelated message.
Proposed assertion
expect(error.message).not.toContain('FES-1004'); expect(error.message).not.toContain('error.flowexecservice'); + expect(error.message).toBe('Invalid flow execution ID provided in the request'); expect(error.code).toBe('FES-1004');🤖 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 `@packages/javascript/src/errors/__tests__/ThunderIDAPIError.test.ts` around lines 181 - 195, Add component-level regression tests for both React and Vue AcceptInvite non-OK /flow/execute paths, verifying the thrown error is a ThunderIDAPIError with the expected origin, stable flow code, statusCode, statusText, and parsed message. In the existing ThunderIDAPIError test, replace the negative message checks with an exact assertion for the expected parsed message while retaining the code assertion.
🤖 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.
Nitpick comments:
In `@packages/javascript/src/errors/__tests__/ThunderIDAPIError.test.ts`:
- Around line 181-195: Add component-level regression tests for both React and
Vue AcceptInvite non-OK /flow/execute paths, verifying the thrown error is a
ThunderIDAPIError with the expected origin, stable flow code, statusCode,
statusText, and parsed message. In the existing ThunderIDAPIError test, replace
the negative message checks with an exact assertion for the expected parsed
message while retaining the code assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 35109dc9-806a-4eed-a4fe-8f474fab5920
📒 Files selected for processing (3)
packages/javascript/src/errors/__tests__/ThunderIDAPIError.test.tspackages/react/src/components/presentation/auth/AcceptInvite/AcceptInvite.tsxpackages/vue/src/components/presentation/accept-invite/AcceptInvite.ts
Purpose
The React and Vue accept invite components threw
new Error("Request failed: " + body), so the raw API response body reached the UI instead of a readable message.Approach
Throw
ThunderIDAPIErrorinstead. It runs the body throughparseApiErrorMessage, so consumers get the resolved message whilecode,statusCodeandstatusTextstay available. A regression test covers the resolved message.Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
🤖 Generated with Claude Code
Summary by CodeRabbit