-
Notifications
You must be signed in to change notification settings - Fork 9
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
New handlers for FetchService #3750
Conversation
@ghsolomon -- thanks again for taking a look, and your comments -- refactoring to try to take account of your comments, I realized that I have a problem with the |
ok @ghsolomon -- can you take another look? Simplified this to deal with just the exception case. |
Looks good to me. One other thought - since we're supporting multiple exception handlers and only allowing developers to append their own, should we move our built-in library handlers out of I.e. |
This provides support I need for being able to catch 401s, in non-SSO based scenario, and respond appropriately by bringing up.
The handler for the successful response seemed like a good thing to add at the same time for symmetry, and we have seen a couple of places where it might have been useful, and avoided the need for overriding FetchService to look for e.g. 200 returns with
success: false
Hoist P/R Checklist
Pull request authors: Review and check off the below. Items that do not apply can also be
checked off to indicate they have been considered. If unclear if a step is relevant, please leave
unchecked and note in comments.
develop
branch as of last change.breaking-change
label + CHANGELOG if so.If your change is still a WIP, please use the "Create draft pull request" option in the split
button below to indicate it is not ready yet for a final review.