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

feat!: Invoke responseMiddleware in error cases as well #372

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

tobilen
Copy link
Contributor

@tobilen tobilen commented Aug 2, 2022

I really like the new middleware feature and would like to use it for some basic error handling.

Unfortunately, the current implementation only invokes the middleware if the fetch request has been resolved, meaning it will never get called on request failure.

BREAKING CHANGE:

Middleware will get either the response or error now, whereas before it would only get the response and never run in the error case.

Copy link
Member

@jasonkuhrt jasonkuhrt left a comment

Choose a reason for hiding this comment

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

This is a breaking change but I'm ok with it.

Please fix the conflicts (just run format script, should do most of the work).

@jasonkuhrt jasonkuhrt changed the title Invoke responseMiddleware in error cases as well feat!: Invoke responseMiddleware in error cases as well Aug 2, 2022
@tobilen tobilen force-pushed the invoke-middleware-on-error branch from 7e54114 to 5de748c Compare August 2, 2022 21:20
@tobilen
Copy link
Contributor Author

tobilen commented Aug 2, 2022

thanks for the quick response! branch should be up to date now

@tobilen
Copy link
Contributor Author

tobilen commented Aug 9, 2022

@jasonkuhrt anything missing?

@jasonkuhrt jasonkuhrt merged commit 2f221a4 into graffle-js:master Aug 9, 2022
@tobilen tobilen deleted the invoke-middleware-on-error branch August 9, 2022 09:07
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