Content is not disposed when request is dispatched
Closed this issue · 10 comments
I have a bug in my code because I attempt to issue the same HttpRequestMessage twice and I want to write a unit test to demonstrate the issue and to show that the code I am about to write indeed solves the issue.
I wrote the following unit test to demonstrate the underlying problem:
[Test]
public void Retry_httpClient()
{
var mockHttp = new MockHttpMessageHandler();
mockHttp.Expect(HttpMethod.Post, "https://api.fictitious-vendor.com/v1/endpoint").Respond("application/json", "{'name' : 'This is a test'}");
mockHttp.Expect(HttpMethod.Post, "https://api.fictitious-vendor.com/v1/endpoint").Respond("application/json", "{'name' : 'This is a test'}");
var request = new HttpRequestMessage(HttpMethod.Post, "https://api.fictitious-vendor.com/v1/endpoint")
{
Content = new StringContent("Hello World!")
};
var httpClient = mockHttp.ToHttpClient();
httpClient.SendAsync(request);
httpClient.SendAsync(request);
}
This unit test fails when it attempts to dispatch the request for the second time with a The request message was already sent. Cannot send the same request message multiple times.
exception. This is excellent: the unit test reproduces the issue my code is experiencing.
My next step is to write a fix for this issue by 'cloning' the request before each attempt. Something like this:
[Test]
public void Retry_httpClient()
{
var mockHttp = new MockHttpMessageHandler();
mockHttp.Expect(HttpMethod.Post, "https://api.fictitious-vendor.com/v1/endpoint").Respond("application/json", "{'name' : 'This is a test'}");
mockHttp.Expect(HttpMethod.Post, "https://api.fictitious-vendor.com/v1/endpoint").Respond("application/json", "{'name' : 'This is a test'}");
var request = new HttpRequestMessage(HttpMethod.Post, "https://api.fictitious-vendor.com/v1/endpoint")
{
Content = new StringContent("Hello World!")
};
var httpClient = mockHttp.ToHttpClient();
httpClient.SendAsync(Clone(request));
httpClient.SendAsync(Clone(request));
}
public static HttpRequestMessage Clone(HttpRequestMessage request)
{
HttpRequestMessage clone = new HttpRequestMessage(request.Method, request.RequestUri)
{
Content = request.Content,
Version = request.Version
};
foreach (var prop in request.Properties)
clone.Properties.Add(prop);
foreach (var header in request.Headers)
clone.Headers.TryAddWithoutValidation(header.Key, header.Value);
return clone;
}
This unit test completed successfully which led me to believe I had solved the issue but when I tried against an actual HTTP server instead of using mockHttp, I received an exception that said I was accessing a disposed object. It took me a while to figure out that the 'object' in question was the content of the request and the root of the issue is that HttpClient disposes the content stream after the request is dispatched and therefore no longer available for the second request.
I was able to fix this problem by cloning the content as well as the request, but my question is: why is it that I didn't get the same exception when running the unit test utilizing mockHttp?
I'm guessing there's a slight difference between how MockHttp and the 'real' HttpClient handle the content when a request is dispatched?
Hi @Jericho, what version of MockHttp are you running?
<PackageReference Include="RichardSzalay.MockHttp" Version="3.2.1" />
The difference is likely due to the magic MockHttp has to due in order to run filters like WithFormData
. Since the
Still, I'll take a look at your repro and see if there's anything that can be done. Requests should always be created from scratch before sending, but I agree that MockHttp should behave the same as the real handler as much as possible.
Thanks Richard. I ran several integration tests against actual HTTP servers and I can reliably demonstrate the issue and I can also demonstrate that cloning the request and the content before dispatching each request solves my problem but it would be great to be able to demonstrate it in unit tests!
@richardszalay Any news on this?
I'm running into the exact same problem - MockHttp 5.0.0 doesn't dispose the request content as Microsoft's HttpClient does, which makes it hard for me to unit test multiple retries on the same request.
edit: Realized I was using v3.2.1 and updated to 5.0, but the issue remains.
Running into the same issue as @stannedelchev attempting to validate call/retry count on the same request.
mockHttpClient.GetMatchCount(request)
always returns 1 even though my subject-under-test has made the request multiple times.
I am using nuget version 5.0.0.
@stannedelchev The issue is actually that the ObjectDisposedException
isn't triggered, not that it isn't disposed. I'll figure out the best way to work this into the core design, but you can work around it for now by adding a custom matcher that consumes the content stream (which will force an exception if it is already disposed):
mockHttp.Expect(HttpMethod.Post, "https://api.fictitious-vendor.com/v1/endpoint")
.With(req => { req.Content.ReadAsStringAsync().Result; return true; })
.Respond("application/json", "{'name' : 'This is a test'}");
@maxxwizard The GetMatchCount
issue seems like a different problem. Can you log a new issue?
@richardszalay i just revisited my offending unit test. i changed enough logic that i cannot figure out how to repro so let's call it user error - mea culpa.
So I've looked into this with a view of fixing it, but I don't actually think there's an issue:
- Attempting to send the same request twice throws an InvalidOperationException (from HttpClient itself)
- Sending a request and then attempting to read from it's
Content
throws an ObjectDisposedException.
I think the problem with the original post here is that the sample code was calling client.SendAsync
twice but without awaiting / blocking either of them so they were happening in parallel. It would make sense in that scenario that the request/content hadn't been disposed since the requests were in flight.
If someone can create me a working repro for this issue I'm happy to fix it but for now I'm going to close it.
Here are the tests I used to check both scenarios:
[Fact]
public async Task Disposes_Content()
{
var mockHandler = new MockHttpMessageHandler();
var client = new HttpClient(mockHandler);
mockHandler
.When(HttpMethod.Post, "/test")
.Respond(HttpStatusCode.OK);
var req = new HttpRequestMessage(HttpMethod.Post, "http://tempuri.org/test")
{
Content = new StringContent("test")
};
await client.SendAsync(req);
Assert.Throws<ObjectDisposedException>(() => req.Content.ReadAsStringAsync().Result);
}
[Fact]
public async Task Cannot_send_request_twice()
{
var mockHandler = new MockHttpMessageHandler();
var client = new HttpClient(mockHandler);
mockHandler
.When(HttpMethod.Post, "/test")
.Respond(HttpStatusCode.OK);
var req = new HttpRequestMessage(HttpMethod.Post, "http://tempuri.org/test")
{
Content = new StringContent("test")
};
await client.SendAsync(req);
Assert.Throws<InvalidOperationException>(() => client.SendAsync(req).Result);
}