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

add error boundary #577

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

SiegfriedBz
Copy link
Contributor

@SiegfriedBz SiegfriedBz commented Sep 10, 2024

handle uncaught exceptions and solves #283

can be tested by throwing an error in any page
throw new Error("This is a simulated error!");

Untitled_.Sep.10.2024.2_24.PM.webm

Next.js doc here

@SiegfriedBz SiegfriedBz requested a review from a team as a code owner September 10, 2024 12:21
Copy link

cloudflare-workers-and-pages bot commented Sep 10, 2024

Deploying dexter with  Cloudflare Pages  Cloudflare Pages

Latest commit: 86a1235
Status: ✅  Deploy successful!
Preview URL: https://04505107.dexter-1we.pages.dev
Branch Preview URL: https://fork-siegfriedbz--add-error.dexter-1we.pages.dev

View logs

@dcts
Copy link
Contributor

dcts commented Sep 10, 2024

Some questions:

  1. When I throw inside the console as described above throw new Error("This is a simulated error!") nothing happens. Is this expected?
  2. Assuming an error gets thrown inside the /trade page, would the UI simply be redirected to "Something went wrong"?
  3. What will happen if you click on "Try again"? Will you be redirected to the page you were on before, or will the action be re-launched somehow?

@dcts dcts mentioned this pull request Sep 10, 2024
@SiegfriedBz
Copy link
Contributor Author

SiegfriedBz commented Sep 11, 2024

@dcts

When using Next.js (also with React alone) I believe it is best practice to implement an error file (at least 1, at the level of the root layout) to handle unexpected runtime errors and display a fallback UI.
The error.tsx file here (in Next.js) is actually making use of React Error Boundary

  1. As the error.tsx is defined as a client-component, to see the console.log This is a simulated error! you need to look in the browser dev tools

  2. As far as I understand the mechanism, this is not a redirect
    If an error is thrown (manually or not) and uncaught in a page, we can see that the url does not change.
    But from this page where this error is thrown and uncaught, this exception "travels/bubbles up" the component tree until it finds an "error boundary" (error.tsx) and then the error.tsx component is rendered and its UI finally painted to the screen. We can add granularity by creating multiple error.tsx files, each at the level of a specific page.

  3. Clicking on "Try again" will make React trying to re-render the current page (no redirect involved). If the error is thrown again, the same error UI will be painted, otherwise the expected component tree will be rendered and the corresponding UI painted to the screen
    The purpose of this error.tsx file is to create a better UX in case an unanticipated / uncaught exception would be thrown, while still being able to log the error for devs

Please have a look at
https://nextjs.org/docs/app/api-reference/file-conventions/error
https://nextjs.org/docs/app/building-your-application/routing/error-handling#uncaught-exceptions

@SiegfriedBz SiegfriedBz reopened this Oct 5, 2024
@fliebenberg
Copy link
Contributor

Where are we with this PR? Not sure if we are waiting on something or just need to review and merge?

@dcts
Copy link
Contributor

dcts commented Nov 8, 2024

Where are we with this PR? Not sure if we are waiting on something or just need to review and merge?

For this PR to merge I suggest to:

  1. improve the styling of the "Something went wrong" text, it looks way too big.
  2. If we have access to the error, we should display it to the user so that they can copy the specifics, which will lead to better bugreports.
  3. we need to test the user experience when uncaught errors happen, and test the "try again" functionality to ensure everything works properly. For that I suggest to mock potential scenarios where this could happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants