Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catching trpc error early before react router error boundary #8757

Merged
merged 12 commits into from
Feb 27, 2025

Conversation

tareq89
Copy link
Collaborator

@tareq89 tareq89 commented Feb 25, 2025

Copy link

Oops! Looks like you forgot to update the changelog. When updating CHANGELOG.md, please consider the following:

  • Changelog is read by country implementors who might not always be familiar with all technical details of OpenCRVS. Keep language high-level, user friendly and avoid technical references to internals.
  • Answer "What's new?", "Why was the change made?" and "Why should I care?" for each change.
  • If it's a breaking change, include a migration guide answering "What do I need to do to upgrade?".

@tareq89 tareq89 self-assigned this Feb 25, 2025
Copy link
Contributor

@Keksike Keksike left a comment

Choose a reason for hiding this comment

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

I tried this out by throwing an error from useEventConfiguration(), but that caused a blank white screen.

Should it show the hint and redirect to login instead, as described on the issue?

@tareq89
Copy link
Collaborator Author

tareq89 commented Feb 25, 2025

I tried this out by throwing an error from useEventConfiguration(), but that caused a blank white screen.

Should it show the hint and redirect to login instead, as described on the issue?

Please check now.

@tareq89 tareq89 requested a review from Keksike February 25, 2025 14:50
@rikukissa
Copy link
Member

@tareq89 could you attach a screenshot of the error view in the PR description please?

@tareq89
Copy link
Collaborator Author

tareq89 commented Feb 27, 2025

@rikukissa
Screenshot 2025-02-27 at 10 46 38 AM

If the page is not broken, then it should show StyledErrorBoundary component. If it is a 401 error, the there should be hint of redirection to login page.

In order for this error to happen, you can simulate some scenarios. like:

  1. Modify the access token with adding or removing some extra character and reload the app for 401 scenario
  2. Throw an error from useEventConfigurations hook and reload the app

@makelicious
Copy link
Collaborator

Good stuff, thank you!

Could you attend to the linting issues so the CI passes

/home/runner/work/opencrvs-core/opencrvs-core/packages/client/src/v2-events/routes/TRPCErrorBoundary.stories.tsx
Warning:   37:11  warning  Missing JSX expression container around literal string: "Normal Render"                                                     react/jsx-no-literals
Warning:   38:10  warning  Missing JSX expression container around literal string: "This content is inside the error boundary and renders correctly."  react/jsx-no-literals
Warning:   57:43  warning  Unexpected any. Specify a different type                                                                                    @typescript-eslint/no-explicit-any

/home/runner/work/opencrvs-core/opencrvs-core/packages/client/src/v2-events/routes/TRPCErrorBoundary.tsx
Error:    21:1   error    '@client/profile/profileActions' import is restricted from being used by a pattern      no-restricted-imports
Warning:    67:5   warning  Unexpected console statement                                                            no-console
Warning:    71:19  warning  'redirectToAuthentication' is already declared in the upper scope on line 21 column 10  no-shadow
Warning:    73:9   warning  Must use destructuring state assignment                                                 react/destructuring-assignment
Warning:    74:21  warning  Must use destructuring state assignment                                                 react/destructuring-assignment
Warning:    99:21  warning  'error' is already declared in the upper scope on line 74 column 13                     no-shadow
Warning:   100:13  warning  Unexpected console statement                                                            no-console
Warning:   140:12  warning  Must use destructuring props assignment                                                 react/destructuring-assignment

@tareq89 tareq89 merged commit c07dff4 into develop Feb 27, 2025
43 checks 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.

4 participants