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

fix(ui): broken error handling for all api requests #1809

Merged

Conversation

shaneikennedy
Copy link
Contributor

@shaneikennedy shaneikennedy commented Jun 4, 2024

fixes: #1807

This is currently affecting all error handling for failed api requests

handleError is not inside a component and so hooks can't be used.

React-router suggests using the redirect function instead of useNavigate but this only works if it's inside a loader/action which I don't think we necessarily are.

Tested and this works, normal error handling shows a toast and if I get a 401/403 im redirected to the login page and i still see a toast with the error message 👍

handleError is not inside a component and so hooks can't be used.

React-router suggests useing the redirect function instead of
useNavigate but this only works if it's inside a loader/action which I
don't think we necessarily are.
@AlexisSouquiere
Copy link
Collaborator

Well done ! I tested the fix, I confirm that errors are correctly shown now. Thanks 🚀

@AlexisSouquiere AlexisSouquiere changed the title fix: broken error handling for all api requests fix(ui): broken error handling for all api requests Jun 5, 2024
@shaneikennedy
Copy link
Contributor Author

@AlexisSouquiere Given that this affects every failed api request for everyone on 0.25 do you think we can get a new version for this?

@AlexisSouquiere
Copy link
Collaborator

AlexisSouquiere commented Jun 5, 2024

I think @tchiotludo agreed in another discussion that we should release more often now. So I assume that after merging this PR and maybe checking few other issues there is in the backlog we will release a patch version

@shaneikennedy
Copy link
Contributor Author

Sounds good, thanks @AlexisSouquiere 🍺

@tchiotludo tchiotludo merged commit 29df54a into tchiotludo:dev Jun 5, 2024
4 checks passed
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.

Regression for handling offset update error
3 participants