Bug: response.RequestMessage is disposed if you get multiple 429 responses from contentful

[patrikholmberg]

Hi.
I've found that we are getting a ObjectDisposedException Cannot access a disposed object. Object name: 'System.Net.Http.StreamContent at this line when we get more than one consecutive 429 response for a request towards the contentful api due to request limit.

contentful.net/Contentful.Core/ContentfulClientBase.cs

Line 332 in 751d123

response = await _httpClient.SendAsync(clonedMessage, HttpCompletionOption.ResponseHeadersRead).ConfigureAwait(false);

I think I understand why and I think it is because of the introduced using statment here

contentful.net/Contentful.Core/ContentfulClientBase.cs

Line 330 in 751d123

using var clonedMessage = await CloneHttpRequest(response.RequestMessage);

If the MaxNumberOfRateLimitRetries is set to allow more than 1 retry, when do the retry we overwrite the response object with a new response based on the clonedMessage. This would result in the response.RequestMessage being a reference to this clonedMessage. And since the introduction of the using statement, this clonedMessage will be disposed directly after the retry which also would result in the response.RequestMessage being disposed as well since it is a reference.
So every time we reach the request limit and get multiple 429 messages, we will get the ObjectDisposedException exception when trying to make a clone out of the response.RequestMessage

So this will always fail if the response of the request get 2 or more 429 responses.

What is it that is failing? It is when the message is being cloned in the CloneHttpRequest method. It is trying to copy the request message content into a MemoryStream here

contentful.net/Contentful.Core/ContentfulClientBase.cs

Line 355 in 751d123

await message.Content.CopyToAsync(ms).ConfigureAwait(false);

Since the request message is being disposed so is this StreamContent object in the message that is being copied.

A possible solution could be to store the response.RequestMessage in a variable at the begining of this method and always clone based of this variable. Something kind of like this:

protected async Task<HttpResponseMessage> EnsureSuccessfulResult(HttpResponseMessage response)
{
	if (!response.IsSuccessStatusCode)
	{
		var requestMessage = response.RequestMessage;
		
		if(response.StatusCode == System.Net.HttpStatusCode.NotModified)
		{
			return response;
		}
		if((int)response.StatusCode == 429 && _options.MaxNumberOfRateLimitRetries > 0)
		{
			//Limit retries to 10 regardless of config
			for (var i = 0; i < _options.MaxNumberOfRateLimitRetries && i < 10; i++)
			{
				try
				{
					await CreateExceptionForFailedRequest(response).ConfigureAwait(false); ;
				}
				catch (ContentfulRateLimitException ex)
				{
					await Task.Delay(ex.SecondsUntilNextRequest * 1000).ConfigureAwait(false);
				}
			   
				using var clonedMessage = await CloneHttpRequest(requestMessage);

				response = await _httpClient.SendAsync(clonedMessage, HttpCompletionOption.ResponseHeadersRead).ConfigureAwait(false);

				if (response.IsSuccessStatusCode)
				{
					return response;
				}
			}
		}

		await CreateExceptionForFailedRequest(response);
	}

	return response;
}