BingAds / BingAds-dotNet-SDK

Other
27 stars 42 forks source link

Microsoft.BingAds.Internal.HttpService Accumulates Headers Indefinitely #99

Closed antoinne85 closed 3 years ago

antoinne85 commented 3 years ago

My company just upgraded to version 13.0.10 of the .NET SDK and we started getting an incredible amount of production issues. After several days of research on our own code, I believe we've found the issue.

Microsoft.BingAds.Internal.HttpService has a static client member of type HttpClient. On calls to UploadFileAsync it executes addHeadersAction(HttpService.client.DefaultRequestHeaders). So whatever headers are to be added get added to the DefaultRequestHeaders and are carried on forever. The end result of this is that the headers for User-Agent, AuthenticationToken, CustomerId, DeveloperToken and AccountId grow and grow over time.

In the [constructor of BulkServiceManager](), the instance property HttpService is initialized to a new HttpService.

At BulkServiceManager.cs:604, in SubmitUploadAsyncImpl(...), the addHeadersAction is defined.

At BulkServiceManager.cs:631, HttpService.UploadFileAsync is called, passing in addHeadersAction.

Stepping into that method you'll find addHeadersAction(client.DefaultRequestHeaders). client, in this case is a static member.

To make a long story short, if you re-use the BulkServiceManager for multiple requests, you'll start accumulating headers and your requests will start failing.

Back in this revision of HttpService things look to be correct. At some point it looks like the locally scoped client variable got promoted to a static member.

antoinne85 commented 3 years ago

Some more research... I think this behavior is specific to 13.0.9+ as it seems to have been introduced here. (That's just speculation based on the commit message.)

qitia commented 3 years ago

hi, thanks for report. I also repro this issue. Please expect version 13.0.11 which will be released soon(middle of August).

deeparsh98 commented 3 years ago

Hi there, We are also getting an exception which originates within PostAsync method of the same service i.e. Microsoft.BingAds.Internal.HttpService. The stack trace looks like this

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
   at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
   at System.Net.Http.Headers.HttpHeaders.AddHeaderInfo(String headerName, HeaderStoreItemInfo sourceInfo)
   at System.Net.Http.Headers.HttpHeaders.AddHeaders(HttpHeaders sourceHeaders)
   at System.Net.Http.Headers.HttpRequestHeaders.AddHeaders(HttpHeaders sourceHeaders)
   at System.Net.Http.HttpClient.PrepareRequestMessage(HttpRequestMessage request)
   at System.Net.Http.HttpClient.SendAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.PostAsync(Uri requestUri, HttpContent content, CancellationToken cancellationToken)
   at Microsoft.BingAds.Internal.HttpService.PostAsync(Uri requestUri, List`1 formValues, TimeSpan timeout)
   at Microsoft.BingAds.Internal.OAuth.UriOAuthService.<GetAccessTokensAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---

Is this already been addressed and if so shall we expect the fix in the coming version 13.0.11...?

qitia commented 3 years ago

13.0.11 was published last Friday, Please try this version.