NullReferenceException from PublishArtifactsInManifest in ASP.NET Core build

[SteveSandersonMS]

• This issue is blocking
• This issue is causing unreasonable pain

The ASP.NET Core build at https://dev.azure.com/dnceng/internal/_build/results?buildId=1373561&view=logs&j=226748d0-f812-5437-d3f0-2dd291f5666e&t=bad11196-972e-5d03-45a8-9db526506073&l=51 failed with the following error:

2021-09-20T23:33:54.2609031Z D:\a\1\s\.packages\microsoft.dotnet.arcade.sdk\7.0.0-beta.21466.4\tools\SdkTasks\PublishArtifactsInManifest.proj(123,5): error : Object reference not set to an instance of an object.
2021-09-20T23:33:54.2709639Z ##[error].packages\microsoft.dotnet.arcade.sdk\7.0.0-beta.21466.4\tools\SdkTasks\PublishArtifactsInManifest.proj(123,5): error : Object reference not set to an instance of an object.
2021-09-20T23:33:54.2722187Z D:\a\1\s\.packages\microsoft.dotnet.arcade.sdk\7.0.0-beta.21466.4\tools\SdkTasks\PublishArtifactsInManifest.proj(123,5): error : Publishing symbols failed : 
2021-09-20T23:33:54.2727023Z ##[error].packages\microsoft.dotnet.arcade.sdk\7.0.0-beta.21466.4\tools\SdkTasks\PublishArtifactsInManifest.proj(123,5): error : Publishing symbols failed : 

This only happened once, so it seems to be an intermittent issue. There are binlogs at https://dev.azure.com/dnceng/internal/_build/results?buildId=1373689&view=artifacts&pathAsName=false&type=publishedArtifacts, in which I see the following:

So, it looks like the underlying issue is that the endpoint to which we're trying to publish didn't respond, or there was some other network issue.

Ideally, PublishArtifactsInManifest would report the underlying issue instead of throwing the NullReferenceException, so that we don't risk blaming PublishArtifactsInManifest for this and can more directly get to the cause without going through binlogs.

[MattGal]

tossing into FR since this is customer pain from code exposed by Arcade that impacts day-to-day operations.

Looking at the call stack:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.SymbolUploader.ArtifactHttpRetryMessageHandler.TraceHttpRequestRetrying(VssTraceActivity activity, HttpRequestMessage request, Int32 attempt, TimeSpan backoffDuration, Nullable`1 httpStatusCode, Nullable`1 webExceptionStatus, Nullable`1 socketErrorCode, Nullable`1 winHttpErrorCode)
   at Microsoft.SymbolUploader.VssHttpRetryMessageHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at Microsoft.SymbolUploader.ArtifactHttpRetryMessageHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
   at Microsoft.SymbolUploader.VssHttpClientBase.SendAsync(HttpRequestMessage message, HttpCompletionOption completionOption, Object userState, CancellationToken cancellationToken)
   at Microsoft.SymbolUploader.VssHttpClientBase.SendAsync[T](HttpRequestMessage message, Object userState, CancellationToken cancellationToken)
   at Microsoft.SymbolUploader.VssHttpClientBase.GetResourceLocationsAsync(Boolean allHostTypes, Object userState, CancellationToken cancellationToken)
   at Microsoft.SymbolUploader.VssHttpClientBase.EnsureResourceLocationsPopulated(Object userState, CancellationToken cancellationToken)
   at Microsoft.SymbolUploader.VssHttpClientBase.GetResourceLocationAsync(Guid locationId, Object userState, CancellationToken cancellationToken)
   at Microsoft.SymbolUploader.VssHttpClientBase.CreateRequestMessageAsync(HttpMethod method, IEnumerable`1 additionalHeaders, Guid locationId, Object routeValues, ApiResourceVersion version, HttpContent content, IEnumerable`1 queryParameters, Object userState, CancellationToken cancellationToken, String mediaType)
   at Microsoft.SymbolUploader.VssHttpClientBase.SendAsync[T](HttpMethod method, IEnumerable`1 additionalHeaders, Guid locationId, Object routeValues, ApiResourceVersion version, HttpContent content, IEnumerable`1 queryParameters, Object userState, CancellationToken cancellationToken)
   at Microsoft.SymbolUploader.PublishOperation.CreateRequestAsync(SymbolHttpClient httpClient, CancellationToken cancellationToken)
   at Microsoft.SymbolUploader.PublishOperation.PerformOperationAsync(IEnumerable`1 fileInfos)
   at Microsoft.SymbolUploader.PublishOperation.PublishFiles(IEnumerable`1 fileInfos)
   at Microsoft.DotNet.Build.Tasks.Feed.PublishSymbolsHelper.PublishAsync(TaskLoggingHelper log, String symbolServerPath, String personalAccessToken, IEnumerable`1 inputPackages, IEnumerable`1 inputFiles, HashSet`1 packageExcludeFiles, Int32 expirationInDays, Boolean convertPortablePdbsToWindowsPdbs, Boolean publishSpecialClrFiles, HashSet`1 pdbConversionTreatAsWarning, Boolean dryRun, Boolean timer, Boolean verboseLogging) in /_/src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishSymbolsHelper.cs:line 90

... everything except the bottom of the stack is actually in external code (Microsoft.SymbolUploader) so we should consult with @mikem8361 to ask about this, but I think in all cases we could simply retry from PublishSymbolsHelper.PublishAsync.

[mikem8361]

It looks like it is attempting to retry an http request, logging the error and something in the log formatting is null. The only thing I can see from inspecting the code that dereferences something is the "activity" parameter. Which means the static property VssTraceActivity.Current is null.

I could make changes to this logging/tracing code to make it more resilient if needed.

/cc: @hoyosjs

[riarenas]

Another failure that looks like an instance of this: https://dev.azure.com/dnceng/internal/_build/results?buildId=1384845&view=results

[MattGal]

I could make changes to this logging/tracing code to make it more resilient if needed.

/cc: @hoyosjs

This would definitely be the preferable fix... it might even end up successfully retrying if so. If that's not feasible to do soon though, can you comment about whether outer code catching the null ref and retrying would be an acceptable workaround?

[mikem8361]

We already have some fixes in PR (and approved) #7964

[riarenas]

I believe #7964 is already out to fix this.