-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[typescript-fetch] Support error handler in middleware #12716
[typescript-fetch] Support error handler in middleware #12716
Conversation
cc @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02) @davidgamero (2022/03) @mkusaka (2022/04) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution
} | ||
} | ||
if (!response) { | ||
throw new FetchError(e, 'Interceptors did not provide an alternative response'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about “The request failed and the interceptors did not return an alternative response?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, changed
@@ -166,7 +166,25 @@ export class BaseAPI { | |||
}) || fetchParams; | |||
} | |||
} | |||
let response = await (this.configuration.fetchApi || fetch)(fetchParams.url, fetchParams.init); | |||
let response = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets use
undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries, done
}) || response; | ||
} | ||
} | ||
if (!response) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!response) { | |
if (response !== undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied
586f0bc
to
4c7b870
Compare
Status Quo
typescript-fetch
provides support for two types of interceptors: to pre-process the request and post-process the response. However, there is one more case which is not handled - network or similar error when response is not received.PR suggestion
This PR suggests an additional interceptor hook
onError
that covers the missing piece. It is called in case of exception during request processing and allows to, for instance, re-try the request, replace it with alternative route, etc.PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.For Windows users, please run the script in Git BASH.
master
(6.0.1) (patch release),6.1.x
(breaking changes with fallbacks),7.0.x
(breaking changes without fallbacks)