Skip to content

feat(extension): implement popup route error boundaries with settings… - #759

Merged
wheval merged 1 commit into
ancore-org:mainfrom
Tobi-8:feat/popup-error-boundary
Jun 6, 2026
Merged

feat(extension): implement popup route error boundaries with settings…#759
wheval merged 1 commit into
ancore-org:mainfrom
Tobi-8:feat/popup-error-boundary

Conversation

@Tobi-8

@Tobi-8 Tobi-8 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

… recovery and safe logging

Description

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📝 Documentation update
  • 🔧 Configuration change
  • ♻️ Code refactoring
  • ⚡ Performance improvement
  • ✅ Test addition/improvement

Security Impact

  • This change involves cryptographic operations
  • This change affects account validation logic
  • This change modifies smart contracts
  • This change handles user private keys
  • This change affects authorization/authentication
  • No security impact

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • E2E tests added/updated (if applicable)

Test Coverage

  • Current coverage: __%
  • New/modified code coverage: __%

Manual Testing Steps

Breaking Changes

  • This PR introduces breaking changes

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings or errors
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published
  • Any flaky-test quarantine entry includes a linked follow-up issue, owner, expiry, and stable failure signature

For High-Security Changes

  • I have documented all security assumptions
  • I have considered attack vectors
  • I have added security-focused test cases
  • I have reviewed against the threat model

Related Issues

Closes #612

Summary by CodeRabbit

Release Notes

  • New Features
    • Added error handling UI that displays when the app encounters issues, preventing blank screens or crashes.
    • Error messages now include a support reference ID to help with troubleshooting.
    • Users can recover from errors by retrying, returning to the home screen, or accessing settings.
    • Sensitive data is automatically redacted from displayed error messages for security.

@drips-wave

drips-wave Bot commented Jun 2, 2026

Copy link
Copy Markdown

@Tobi-8 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a React error boundary component to the extension wallet popup that catches unhandled render errors, sanitizes sensitive content in error messages, generates unique error identifiers for support reference, provides a fallback UI with recovery options, and integrates recovery navigation into the main router.

Changes

Error Boundary and Fallback Components

Layer / File(s) Summary
Error boundary and fallback components
apps/extension-wallet/src/components/ErrorBoundary/ErrorBoundary.tsx, apps/extension-wallet/src/components/ErrorBoundary/ErrorFallback.tsx
ErrorBoundaryProps and ErrorFallbackProps contracts define reset, navigation, and report callbacks. sanitizeMessage() redacts Stellar secret keys and patterns like private keys/mnemonic/seed/passphrase. generateErrorId() produces compact ERR-timestamp-random identifiers. ErrorBoundary catches render errors, logs sanitized-only payloads with errorId, and renders ErrorFallback UI. ErrorFallback displays error details, support reference, invokes optional onReport callback via useEffect, and renders conditional "Go to home" / "Go to settings" buttons plus "Try again" reset button.
Router integration
apps/extension-wallet/src/router/index.tsx
ExtensionRouterContent imports ErrorBoundary and wraps the entire Routes tree with error boundary, adding useNavigate() hook and onGoHome/onGoToSettings callbacks that navigate to /home or /settings with replace: true.
Comprehensive test coverage
apps/extension-wallet/src/components/ErrorBoundary/__tests__/ErrorBoundary.test.tsx
Test suite covers: renders children normally without errors, catches render errors and displays fallback with thrown message and generated error ID, resets boundary and recovers when "Try again" is clicked, navigation callbacks trigger from fallback buttons, sensitive data is redacted from DOM and console.error logs (including Stellar secret key pattern), and onReport is invoked with errorId and sanitized message. Tests suppress React error logging, use Bomb component to trigger errors, and harness helper to mount boundary inside MemoryRouter.

Sequence Diagram

sequenceDiagram
  participant User as User/Extension
  participant Routes as Routes
  participant ErrorBoundary as ErrorBoundary
  participant Fallback as ErrorFallback UI
  participant Logger as Console/Logger
  
  User->>Routes: navigate or interact
  Routes->>Routes: render child component
  Routes->>ErrorBoundary: throw uncaught error
  ErrorBoundary->>ErrorBoundary: sanitizeMessage()
  ErrorBoundary->>ErrorBoundary: generateErrorId()
  ErrorBoundary->>Logger: log sanitized message + errorId
  ErrorBoundary->>Fallback: render with error & errorId
  Fallback->>Logger: invoke onReport(errorId, message)
  User->>Fallback: click "Go to home"
  Fallback->>Fallback: call resetErrorBoundary()
  Fallback->>Routes: navigate to /home
  Routes->>Routes: render home route
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A boundary with grace so fine,
Catches errors in no time,
Sanitizes secrets, no key in sight,
Generates IDs through the night,
Home or settings, the user can roam—
Error boundary brings them safely home! 🏠

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title describes the main feature (popup route error boundaries with settings recovery), though it's truncated and incomplete.
Linked Issues check ✅ Passed The code implements all acceptance criteria from issue #612: error boundaries catch render errors, ErrorFallback provides reset/navigation actions, error IDs enable support correlation, logging is sanitized, and tests verify recovery.
Out of Scope Changes check ✅ Passed All changes are scoped to the required directories (apps/extension-wallet/src/components/ErrorBoundary and apps/extension-wallet/src/router) and directly support error boundary implementation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/extension-wallet/src/components/ErrorBoundary/__tests__/ErrorBoundary.test.tsx`:
- Around line 120-126: The test "redacts Stellar secret key (56 chars starting
with S)" uses an invalid key with em-dashes so the Stellar regex never matches
and the test passes for the wrong reason; replace the test string assigned to
stellarSecretKey in ErrorBoundary.test.tsx with a 56-character base32 string
that begins with 'S' and contains only characters A-Z and 2-7 (e.g., 'S' + 55
characters from [A-Z2-7]) so the redaction logic (the regex in
SENSITIVE_PATTERNS / the Stellar redaction case inside the ErrorBoundary/Bomb
rendering path) is actually exercised by harness(<Bomb ... />) and the
assertions checking that the original key is not in the document and "redacted"
is present remain valid.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 63b26b2b-a29f-4d0f-be73-2463fff81047

📥 Commits

Reviewing files that changed from the base of the PR and between 3d6e31f and 6da4b39.

📒 Files selected for processing (4)
  • apps/extension-wallet/src/components/ErrorBoundary/ErrorBoundary.tsx
  • apps/extension-wallet/src/components/ErrorBoundary/ErrorFallback.tsx
  • apps/extension-wallet/src/components/ErrorBoundary/__tests__/ErrorBoundary.test.tsx
  • apps/extension-wallet/src/router/index.tsx

@Tobi-8

Tobi-8 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Given the very limited time available, I used AI assistance to complete the task before the deadline. I understand if that's a factor in evaluation, but I'd appreciate consideration of the time constraints under which the submission was made.

@wheval
wheval merged commit 17b4828 into ancore-org:main Jun 6, 2026
1 check passed
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.

[EXTENSION] Add error boundary fallback for popup routes

2 participants