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.

Sign up for GitHub

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

Jump to bottom

Add error and catch boundaries #554

Merged
merged 6 commits into from
Apr 11, 2023
Merged

Add error and catch boundaries #554

merged 6 commits into from
Apr 11, 2023

Conversation

Myrfion
Copy link
Contributor

@Myrfion Myrfion commented Apr 10, 2023

  • add a component for templating seen errors (the kind of you get in CatchBoundry) with default layout and error messages with possibility to customize error messages
  • add a component for unseen errors (the kind you get unexpectedly or when your throw new Error), it has some layout and accepts custom text as prop
  • Put error and catch boundaries for both DNS records and certificate routes + root catch and error boundary

image

@Myrfion Myrfion self-assigned this Apr 10, 2023
@Myrfion Myrfion added this to the Milestone 1.0 milestone Apr 10, 2023
@Myrfion
Copy link
Contributor Author

Myrfion commented Apr 10, 2023

Closes #391

humphd
humphd requested changes Apr 10, 2023
View reviewed changes
app/routes/__index/certificate/index.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/$dnsRecordId.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/index.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/index.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/index.tsx Outdated Show resolved Hide resolved
@Myrfion
Copy link
Contributor Author

Myrfion commented Apr 10, 2023

Closes #445

This was linked to issues Apr 10, 2023
Error when dns-records/:id is not a valid ID #445
Closed
Add error/catch boundaries in Remix to deal with server-side errors #391
Closed
@Myrfion Myrfion requested a review from humphd April 10, 2023 15:00
@humphd
Copy link
Contributor

humphd commented Apr 10, 2023

Let's confirm that the steps in #445 are fixed by this (i.e., that we get back some useful error UI vs a crash stack).

@Eakam1007
Copy link
Contributor

Eakam1007 commented Apr 10, 2023 •
edited
Loading

Looks like that error is being handled now:
image
I wonder if we want to capitalize record and id in the message though?

Eakam1007
Eakam1007 requested changes Apr 10, 2023
View reviewed changes
Copy link
Contributor

@Eakam1007 Eakam1007 left a comment

Choose a reason for hiding this comment

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

A few small things, rest looks good to me

app/routes/__index/certificate/index.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/index.tsx Outdated Show resolved Hide resolved
@Myrfion Myrfion requested review from humphd and Eakam1007 April 10, 2023 23:55
@Myrfion Myrfion merged commit 5664897 into main Apr 11, 2023
@Myrfion Myrfion deleted the feature/error-boundry branch April 11, 2023 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Eakam1007 Eakam1007 Eakam1007 approved these changes

@humphd humphd humphd approved these changes

@Ririio Ririio Awaiting requested review from Ririio

Assignees

@Myrfion Myrfion

Labels
None yet
Projects
None yet
Milestone
Milestone 1.0
3 participants
@Myrfion @humphd @Eakam1007