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

45 Handles errors using react error boundaries #111

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

Conversation

jackcruz53
Copy link
Contributor

No description provided.

@jackcruz53 jackcruz53 requested a review from charliepnnl July 19, 2023 22:00
@jackcruz53 jackcruz53 marked this pull request as ready for review July 19, 2023 22:00
Copy link
Contributor

@charliepnnl charliepnnl left a comment

Choose a reason for hiding this comment

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

I like the idea of taking advantage of the router errorElement for route-level error messages.

I just have a few comments that need to be addressed.

@@ -0,0 +1,13 @@
import React from "react"

const errorElement = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about calling this component RouterErrorElement?

Please add jsDoc documentation for this component. Most important is to explain how it is to be used.

This component should also follow the Typescript pattern of our other components – see home.tsx.


const errorElement = () => {
return (
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

How about the following message:

<h1>An Unexpected Error Has Occurred</h1>
<p>
    To recover, try the following:
    <ul>
        <li>Refresh the browser</li>
        <li>Go back to the <a href="/">home screen</a></li>
    </ul>
</p>

When we have a support email address, we can add another bullet about contacting us.

@charliepnnl
Copy link
Contributor

My testing shows that React Router also sends the stack trace to the console as well as displaying the error component. Bonus!

@charliepnnl
Copy link
Contributor

We should also pursue adding ErrorBoundaries in other places such as:

  • Around the content of RootLayout so the banner can continue to be displayed
  • For PhotoInput and Photo to display a more helpful error message when an error occurs related to images
  • Other places?

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.

2 participants