Handle nested failures in HttpRequestException #1816
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
winstliu
approved these changes
Oct 23, 2024
The v2 version of `upload-artifact`/`download-artifact` is no longer supported and stopped working. Let's update to v4, which _does_ work. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Every once in a while, the GitHub Actions we use (such as `checkout`, `upload-artifact`, etc) require updates as the old versions stop working. Let's ask Dependabot to take care of this tedious task. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Merged
`HttpClientHandler` will automatically resubmit an HTTP request that fails in certain circumstances. One such circumstance is when `UseDefaultCredentials` is set to `true``, alternative credentials were provided on the request, and the request failed with "401 Unauthorized", then it will resubmit using the default credentials. If an exception is thrown when getting the default credentials that exception is not handled, but is wrapped in `HttpRequestExceptio`n and thrown by `SendAsync` instead of returning the original response with the failing status code. See the following code snippet from `HttpWebRequest`: https://referencesource.microsoft.com/#System/net/System/Net/HttpWebRequest.cs,5535 When this happens in GVFS, the result is that GVFS does not recognize the failure as being authentication related, so it does not refresh the credential. Instead, it loops through all its retries, and eventually fails the request. This is typically visible to users as a file system exception (e.g. file not found) if the GVFS trigger was accessing a virtual file or other operation on an individual file, or various errors (including "this repository requires the GVFS protocol") for a `git pull`. The symptoms will continue for the user until they remount the GVFS enlistment, which forces GVFS to refresh its credential. This commit adds an exception handler for `HttpRequestException`` to `client.SendAsync`, which attempts to find the original failed response embedded in the inner exception properties. If it does so, it logs a warning and continues processing using the original failed response, which will trigger the logic for handling the various possible status codes. If it can't extract the original failed response, then it will let the exception bubble up. Signed-off-by: Tyrie Vella <tyrielv@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
HttpClientHandlerwill automatically resubmit an HTTP request that fails in certain circumstances. One such circumstance is whenUseDefaultCredentialsis set to true, alternative credentials were provided on the request, and the request failed with 401 Unauthorized, then it will resubmit using the default credentials. If an exception is thrown when getting the default credentials that exception is not handled, but is wrapped in HttpRequestExceptionand thrown bySendAsyncinstead of returning the original response with the failing status code. SourceWhen this happens in GVFS, the result is that GVFS does not recognize the failure as being authentication related, so it does not refresh the credential. Instead, it loops through all its retries, and eventually fails the request. This is typically visible to users as a file system exception (e.g. file not found) if the GVFS trigger was accessing a virtual file or other operation on an individual file, or various errors (including "this repository requires the GVFS protocol") for a git pull. The symptoms will continue for the user until they remount the GVFS enlistment, which forces GVFS to refresh its credential.
This pull request adds an exception handler for HttpRequestException to
client.SendAsync, which attempts to find the original failed response embedded in the inner exception properties. If it does so, it logs a warning and continues processing using the original failed response, which will trigger the logic for handling the various possible status codes. If it can't extract the original failed response, then it will let the exception bubble up.