-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Convert symbol-search crash to NFW #65279
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -511,23 +511,22 @@ private async Task<XElement> DownloadFileAsync(string serverPath, CancellationTo | |
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
||
| var resultOpt = await TryDownloadFileAsync(client, cancellationToken).ConfigureAwait(false); | ||
| if (resultOpt == null) | ||
| var (element, delay) = await TryDownloadFileAsync(client, cancellationToken).ConfigureAwait(false); | ||
| if (element == null) | ||
| { | ||
| var delay = _service._delayService.CachePollDelay; | ||
| await LogInfoAsync($"File not downloaded. Trying again in {delay}", cancellationToken).ConfigureAwait(false); | ||
| await Task.Delay(delay, cancellationToken).ConfigureAwait(false); | ||
| } | ||
| else | ||
| { | ||
| // File was downloaded. | ||
| return resultOpt; | ||
| return element; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary>Returns 'null' if download is not available and caller should keep polling.</summary> | ||
| private async Task<XElement> TryDownloadFileAsync(IFileDownloader fileDownloader, CancellationToken cancellationToken) | ||
| private async Task<(XElement? element, TimeSpan delay)> TryDownloadFileAsync(IFileDownloader fileDownloader, CancellationToken cancellationToken) | ||
| { | ||
| await LogInfoAsync("Read file from client", cancellationToken).ConfigureAwait(false); | ||
|
|
||
|
|
@@ -536,7 +535,7 @@ private async Task<XElement> TryDownloadFileAsync(IFileDownloader fileDownloader | |
| if (stream == null) | ||
| { | ||
| await LogInfoAsync("Read file completed. Client returned no data", cancellationToken).ConfigureAwait(false); | ||
| return null; | ||
| return (element: null, _service._delayService.CachePollDelay); | ||
| } | ||
|
|
||
| await LogInfoAsync("Read file completed. Client returned data", cancellationToken).ConfigureAwait(false); | ||
|
|
@@ -554,28 +553,23 @@ private async Task<XElement> TryDownloadFileAsync(IFileDownloader fileDownloader | |
|
|
||
| // This code must always succeed. If it does not, that means that either the server reported bogus data | ||
| // to the file-downloader, or the file-downloader is serving us bogus data. In other event, there is | ||
| // something wrong with those components, and we should both report the issue to watson, and stop doing | ||
| // something wrong with those components, and we should both report the issue to Watson, and stop doing | ||
| // the update. | ||
| try | ||
| { | ||
| using var reader = XmlReader.Create(stream, settings); | ||
|
|
||
| var result = XElement.Load(reader); | ||
| var element = XElement.Load(reader); | ||
| await LogInfoAsync("Converting data to XElement completed", cancellationToken).ConfigureAwait(false); | ||
| return result; | ||
| } | ||
| catch (Exception e) when (ReportAndDoNotCatch(e)) | ||
| { | ||
| throw ExceptionUtilities.Unreachable(); | ||
| return (element, delay: default); | ||
| } | ||
|
|
||
| bool ReportAndDoNotCatch(Exception exception) | ||
| catch (Exception e) when (_service._reportAndSwallowExceptionUnlessCanceled(e, cancellationToken)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's with the _service. thing here having a function we're calling as opposed to just using the existing static methods?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (this is not blocking for taking this in to 17.4 of course)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same q
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was for testing purposes. we actually validate these codepaths in tests, and it allows the test system to inject a helper that fails-fast as opposed to gracefully falling over (like the main code does). |
||
| { | ||
| // Directly report issue here so we can collect a dump that indicates precisely what is in the | ||
| // stream. Then return 'false' explicitly so that the catch handler doesn't run and the exception | ||
| // properly tears down the update. | ||
| _service._reportAndSwallowExceptionUnlessCanceled(exception, cancellationToken); | ||
| return false; | ||
| // We retrieved bytes from the server, but we couldn't make parse it an xml. out of it. That's very | ||
| // bad. Just trying again one minute later isn't going to help. We need to wait until there is | ||
| // good data on the server for us to download. | ||
| await LogInfoAsync($"Unable to parse file as XElement", cancellationToken).ConfigureAwait(false); | ||
| return (element: null, _service._delayService.CatastrophicFailureDelay); | ||
| } | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.