Support HttpContext.RequestAborted in TestServer

[tmds]

Request to be able to test aborted requests.

HttpContext.RequestAborted should be cancelled when:

• the CancellationToken passed to SendAsync is cancelled
• the HttpMessageHandler is Disposed

[Tratcher]

Note that disposing the HttpClient calls HttpClient.CancelPendingRequests(), so we don't have to do anything special for that case. We just need to pass the CancellationToken through.

[tmds]

I tried implementing the SendAsync-case.

In the ClientHandler, where the HttpContext is created. I set RequestAborted to be equal to the cancellationToken.

To test this I made a Website where I added a SemaphoreSlim which does a WaitAsync using the RequestAborted. I never release the SemaphoreSlim, so the only way it will unblock is by cancelling the token.

Now when I cancel the token, the SemaphoreSlim unblocks by throwing an Exception. This is okay. What is unexpected to me is that the exception is of type ObjectDisposedException and not of type OperationCancelledException.

I saw that the CancellationTokenSource of RequestAborted is different than the one I create for the SendAsync call. The HttpClient uses it in a LinkedTokenSource.

It is not clear to me why this is throwing an ObjectDisposedException instead of an OperationCancelledException. Any thoughts?

[Tratcher]

It's hard to guess without seeing the test code and the exception stack trace.

[tmds]

I made a repo with a test here: https://github.com/tmds/RequestAbortedTestCase.git

This is the small change made to the ClientHandler: https://github.com/tmds/RequestAbortedTestCase/blob/master/src/Microsoft.AspNet.TestHost/ClientHandler.cs#L120

These are the 3 things I observe:

When running the TestWebsite project and pressing the stop button in the browser, an OperationCancelledException is caught here: https://github.com/tmds/RequestAbortedTestCase/blob/master/src/TestWebSite/Startup.cs#L36. This is the behaviour I would like to get when using the TestServer.

When I run the test in 'debug', the call to aquire the semaphore does not return.

When I set a breakpoint in the WebSite at this line and step through the code, the call to the semaphore returns and I get an ObjectDisposedException which says the 'The CancellationTokenSource has been disposed.'.

[tmds]

@Tratcher , were you able to reproduce this?
In the repo with the test, there isn't much code, these are the important bits:

• WebSite
• Test
• Assignment of RequestAborted in ClientHandler

[Tratcher]

I need to get beta7 finished this week. I can take a look next week.

[Tratcher]

Now I see your problem: https://github.com/tmds/RequestAbortedTestCase/blob/master/src/Tests/TestCase.cs#L21
var response = await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cts.Token);
HttpClient internally creates its own CTS linked to the one you passed in (for timeouts). However, that CTS is only good for the duration of SendAsync and then it is disposed. When you use the ResponseHeadersRead parameter SendAsync completes and the CTS is disposed as soon as the response headers are received, so when you try to use it afterwards you get that ODE.

Your cancellation code works in the common cases, you just need a test pattern that doesn't rely on ResponseHeadersRead / streamed responses.

[tmds]

Thanks for looking at this and figuring out where the ODE comes from!
I think most tests with RequestAborted will want to use ResponseHeadersRead or streamed responses to control when the request is aborted. At least, that's the case for the tests I want to write.

[tmds]

Perhaps we can override HttpResponseMessage.Dispose to generate RequestAborted in the TestServer?

[Tratcher]

That sounds promising. You could create a linked CTS so that it was attached to both the CT from SendAsync and HRM.Dispose. Note you'll only want to fire the CT from Dispose if the response has not completed.

[Tratcher]

You can probably do this in the ResponseStream class here: https://github.com/aspnet/Hosting/blob/dev/src/Microsoft.AspNet.TestHost/ClientHandler.cs#L186

[tmds]

@Tratcher I considered a number of alternatives and finally implemented this: https://github.com/tmds/RequestAbortedTestCase/commit/40b97b0e38c8e6c0a4f66c838f4d8b4dc9c7983f

The core is in these lines of code. The Abort logic hasn't changed, which means that if you do a read after the Dispose you get the Exception that caused the pipeline to stop. This also is a way of waiting for the pipeline to finish, which can be handy since the rest of the test may depend on this.

If you think the solution is good, I can make a pull request.

[Tratcher]

That looks feasible.

[tmds]

I worked this out further.

One other thing that became clear, is that the API wait for the pipeline to end (and check what exception caused the pipeline to end) became rather convoluted. See these lines of code.
I thought it would be nicer if the API would be an extention method of HttpResponseMessage, something like: response.PipelineCompleteAsync().

To implement this I would create a derived class of HttpResponseMessage and provide it with an instance of the RequestState. The extention methods would be instance methods of the new class and the HttpResponseMessageExtentions would cast to that new class and invoke the method.

Does this sound like a proper way to add this feature?