Skip to content
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

If HttpResponseMessage is disposed next request fails #33

Closed
hecsalazar opened this issue May 16, 2017 · 13 comments
Closed

If HttpResponseMessage is disposed next request fails #33

hecsalazar opened this issue May 16, 2017 · 13 comments

Comments

@hecsalazar
Copy link

hecsalazar commented May 16, 2017

It would be nice to have an option to make it return a new HttpResponseMessage on every call.
I have a code that disposes the message after extracting the contents and unit tests broke because the response instance gets disposed inside the mock.

System.ObjectDisposedException occurred
Message: Exception thrown: 'System.ObjectDisposedException' in System.Net.Http.dll
Additional information: Cannot access a disposed object.

@richardszalay
Copy link
Owner

Can you provide a reproducible example? The following succeeds for me:

var mockHttp = new MockHttpMessageHandler();
            
var mockedRequest = mockHttp.When("/path");
mockedRequest.Respond(req => new StringContent("test", Encoding.UTF8, "text/plain"));

var request = new HttpRequestMessage(HttpMethod.Post, "http://www.tempuri.org/path?apple=red&pear=green")
{
    Content = new StringContent("testing")
};

string contentA;
using (var responseA = mockedRequest.SendAsync(request, CancellationToken.None).Result)
{
    contentA = responseA.Content.ReadAsStringAsync().Result;
}

string contentB;
using (var responseB = mockedRequest.SendAsync(request, CancellationToken.None).Result)
{
    contentB = responseB.Content.ReadAsStringAsync().Result;
}

@richardszalay
Copy link
Owner

Hi there, I haven't heard back from you on this issue for a while so I'm going to close it. Please feel free to create a new issue if you manage to create a reproducible example.

@hecsalazar
Copy link
Author

hecsalazar commented Jun 7, 2017 via email

@richardszalay
Copy link
Owner

I'm going to close this issue due to lack of activity. If you're able to create a "MCV" example, I'm more than happy to dive deeper and fix the underlying issue.

@chengbo
Copy link

chengbo commented Aug 18, 2017

Hello, I have a reproducible example below

using System.Net.Http;
using RichardSzalay.MockHttp;

namespace MockHttpClient
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Hello World!");

            var mockHandler = new MockHttpMessageHandler();
            mockHandler.When("*").Respond("text/html", "");

            var regularHandler = new HttpClientHandler();
            var repository = new Repository(mockHandler);
            // Use below handler works fine.
            // var repository = new Repository(regularHandler);
            Console.WriteLine(repository.Get1());
            Console.WriteLine(repository.Get2());
        }
    }

    class Repository
    {
        private readonly HttpMessageHandler handler;

        public Repository(HttpMessageHandler handler)
        {
            this.handler = handler;
        }

        public string Get1()
        {
            HttpClient client = new HttpClient(handler, false);
            return client.GetStringAsync("https://www.google.com").Result;
        }

        public string Get2()
        {
            HttpClient client = new HttpClient(handler, false);
            return client.GetStringAsync("https://github.com").Result;
        }
    }
}

Below is the output.

/tmp/MockHttpClient » dotnet run .                                                                                           chengbo@Bo-MBP
Hello World!


Unhandled Exception: System.AggregateException: One or more errors occurred. (Cannot access a disposed object.
Object name: 'System.Net.Http.HttpResponseMessage'.) ---> System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'System.Net.Http.HttpResponseMessage'.
   at System.Net.Http.HttpResponseMessage.CheckDisposed()
   at System.Net.Http.HttpResponseMessage.set_RequestMessage(HttpRequestMessage value)
   at RichardSzalay.MockHttp.MockHttpMessageHandler.<>c__DisplayClass13_0.<SendAsync>b__1(Task`1 resp)
   at System.Threading.Tasks.ContinuationResultTaskFromResultTask`2.InnerInvoke()
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at System.Net.Http.HttpClient.<FinishSendAsyncUnbuffered>d__59.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at System.Net.Http.HttpClient.<GetStringAsyncCore>d__27.MoveNext()
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
   at MockHttpClient.Repository.Get2() in /private/tmp/MockHttpClient/Program.cs:line 42
   at MockHttpClient.Program.Main(String[] args) in /private/tmp/MockHttpClient/Program.cs:line 20

MacOS 10.12.6
Dotnet Core 2.0

@richardszalay richardszalay reopened this Aug 18, 2017
@richardszalay
Copy link
Owner

richardszalay commented Aug 19, 2017

Oddly I still can't reproduce the problem. I'm on Windows, but it works against both .NET and .NET Core.

Am I correct in assuming you're using MockHttp 1.5.0? It sounds like something that would have been a problem before 1.3.1

@chengbo
Copy link

chengbo commented Aug 21, 2017

Thanks for pointing out this, I've just checked my .csproj file, there was

<PackageReference Include="RichardSzalay.MockHttp" Version="3.0.0" />

in there.

Then I used below command to force install 1.5.0, everything works fine.

dotnet add package RichardSzalay.MockHttp --version 1.5.0

I don't know why 3.0.0 is installed without version parameter.

@richardszalay
Copy link
Owner

That's very strange. 3.0.0 was published in error in place of 1.3.0 but was unlisted so it shouldn't have been accepted unless explicitly requested.

@chengbo
Copy link

chengbo commented Aug 22, 2017

Yes, and it would be great if you could add a note for this in README file to prevent others from installing a wrong version without specifying version parameter.

@richardszalay
Copy link
Owner

Done, and I've also logged a bug with the dotnet cli team: https://github.com/dotnet/cli/issues/7492

@hecsalazar
Copy link
Author

hecsalazar commented Aug 22, 2017

Hi Richard,

I confirm I have version 1.5.0, my code is a bit different, since I do an explicit dispose on the HttpResponseMessage after extracting the content:

public async Task HttpSendAsync(HttpRequestMessage requestMessage, X509Certificate2 sslCertificate = null)
{
var httpClient = GetHttpClient(sslCertificate);

        return await CallWithCircuitBreakerAsync(requestMessage.RequestUri.AbsoluteUri, async () =>
        {

            HttpResponseMessage httpResponse = null;
            try
            {
                httpResponse = await httpClient.SendAsync(requestMessage);
                var responseContent = await GetContentAsync(httpResponse);

                return responseContent;
            }
            catch (Exception e)
            {
                logger.Warn($"httpClient.HttpSendAsync failed. Method: {requestMessage.Method} Url: {requestMessage.RequestUri.AbsoluteUri} Error: {e.Message}",e);
                throw;
            }
            finally
            {
                // Dispose response object to avoid memory leak
                httpResponse?.Dispose();
            }

        });

    }

@richardszalay richardszalay reopened this Aug 22, 2017
@richardszalay
Copy link
Owner

I can now reproduce the issue, and I'll look at patching it. FYI there's no need to dispose the response unless you are keeping a reference to it somewhere.

var handler = new MockHttpMessageHandler();

handler.When("*").Respond(HttpStatusCode.OK);

var client = handler.ToHttpClient();

var responseA = client.GetAsync("https://localhost/").GetAwaiter().GetResult();
responseA.Dispose();

var responseB = client.GetAsync("https://localhost/").GetAwaiter().GetResult();

@richardszalay
Copy link
Owner

This is fixed in 1.5.1 (just released)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants