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

Initial error handling implementation #431

Merged
merged 19 commits into from
Jun 24, 2024
Merged

Initial error handling implementation #431

merged 19 commits into from
Jun 24, 2024

Conversation

roll
Copy link
Collaborator

@roll roll commented Jun 18, 2024


In this PR we start catching all the possible server errors and reflect them in the UI (exact UI can be updated later). Currently, error handling in actions is quite verbose but can be improved in #430 (using some kind of error boundaries in actions). Also, this iteration doesn't fully cover nested action errors (still can get into an inconsistent state), which also needs to be addressed in #430.

Other follow-up issues:

Copy link

cloudflare-workers-and-pages bot commented Jun 18, 2024

Deploying opendataeditor with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2bdd46e
Status: ✅  Deploy successful!
Preview URL: https://0f633640.opendataeditor.pages.dev
Branch Preview URL: https://295-error-handling.opendataeditor.pages.dev

View logs

@roll roll requested review from pdelboca and guergana June 18, 2024 14:18
Copy link
Member

@pdelboca pdelboca left a comment

Choose a reason for hiding this comment

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

Looks good @roll ! It reminds me a lot to go pattern:

result, err := doSomething()
if err != nil {
    log.Fatal(err)
}

Note: Is is truly necessary to repeat all the Error.tsx components?

@roll
Copy link
Collaborator Author

roll commented Jun 24, 2024

@pdelboca
Thanks!

Looks good @roll ! It reminds me a lot to go pattern:

Yea, exactly. Still not sure if it's the best one as it's great without nesting but if you have nested actions (e.g. doSomethign -> loadFields -> client.load) you need to have signature on the actions as well. I'll investigate further

Note: Is is truly necessary to repeat all the Error.tsx components?

I think it will be eliminated in #430; it will have an error state only in one store so the error component will be global as well

@roll roll merged commit 9153200 into main Jun 24, 2024
8 checks passed
@roll roll deleted the 295/error-handling branch June 24, 2024 07:45
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.

Better error handling on the client
2 participants