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

unify the query error handling #1176

Open
ollibowers opened this issue Jul 30, 2024 · 0 comments
Open

unify the query error handling #1176

ollibowers opened this issue Jul 30, 2024 · 0 comments
Labels
👤 closed for help 👥 Closed to external help, team only 📝 feature 🧠 New feature or request 🫣 331 🤫 Is only in scope within the current 331 branch

Comments

@ollibowers
Copy link
Collaborator

ollibowers commented Jul 30, 2024

(Probably blocked by useQuery unification and the notification hook, maybe can work on it together)

Currently when an API request fails, we have a pretty inconsistent mix of console.error, throwing, and notifications.
This needs to be unified. Ideally, we will show an antd notification for certain errors, and throw to error boundary on others.

The main errors and how I believe they should be handled are as follows:

  • 400 Bad Request (body had wrong data): BAD, OUR FAULT, error boundary
  • 401 Unauthorized (token is bad, potentially from logout on another tab): notif
  • 403 Forbidden (token is good, but user is not setup, like adding to unplanned): notif
  • 404 Not Found: BAD, OUR FAULT, error boundary
  • 422 Unprocessable Content (pydantic cried): BAD, OUT FAULT, error boundary
  • 500 Internal Server Error: BAD, OUR FAULT, error boundary
  • anything else, probs error boundary??? maybe notification??? discuss with directors

Note that if any of the notification ones happen on a useQuery (when we are getting data), then its a bit unclear how it should be further handled with the absense of data. Discuss these cases with directors as they come up.

The notification should be consistent, and have info about what went wrong. You can investigate throwOnError in react query to filter out the non throws, then do some logic to handle when it doesnt throw. This can also be set globally in the App.tsx where the query client is created.

@ollibowers ollibowers added 📝 feature 🧠 New feature or request 👤 closed for help 👥 Closed to external help, team only 🫣 331 🤫 Is only in scope within the current 331 branch labels Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👤 closed for help 👥 Closed to external help, team only 📝 feature 🧠 New feature or request 🫣 331 🤫 Is only in scope within the current 331 branch
Projects
None yet
Development

No branches or pull requests

2 participants