-
Notifications
You must be signed in to change notification settings - Fork 244
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
Override the default error handler when custom error handler is defined #44
Conversation
Otherwise there is the possibility that they are conflicting with each other. Also added a option, where you can disable the errorHandler at all (for example if you work with the responseInterceptor)
lib/index.js
Outdated
@@ -19,6 +19,7 @@ module.exports = function nuxtAxios (moduleOptions) { | |||
proxyHeaders: true, | |||
proxyHeadersIgnore: ['accept', 'host'], | |||
debug: false, | |||
disableErrorHandler: false, |
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.
What's your idea changing it to errorHandler: true
?
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.
Take a look at #33. Maybe you want to handle your errors together with some other logic in the responseInterceptor. With the current design the responseInterceptor
does not get called if an error happens.
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.
It's just a suggestion. I can also remove this.
For me it works fine, when I'm overriding the default error handler with the custom error handler I defined.
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.
Changes in this PR looks good to me. Just think this option name may be little confusing for users ... Maybe either :
- A new option like
disableDefaultErrorHandler
- Handle explicit
false
value in else condition with:else if (options.disableErrorHandler === false)
BTW thanks for working on this :)
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.
Yeah you are right. I will change this on occasion.
No problem, I‘m thankful for the Package :)
Renamed the option to `disableDefaultErrorHandler`. Added documention for the new option.
I've just updated the PR according to your review. In addition I added a small section documenting the new flag/option. |
Otherwise there is the possibility that they are conflicting with each other.
Also added a option, where you can disable the errorHandler at all (for example if you want to work with the responseInterceptor)