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

Support sending requests with CancellationToken #262

Closed
bgold09 opened this issue Jul 27, 2016 · 15 comments
Closed

Support sending requests with CancellationToken #262

bgold09 opened this issue Jul 27, 2016 · 15 comments
Assignees
Milestone

Comments

@bgold09
Copy link

bgold09 commented Jul 27, 2016

When testing the full server stack, it can be useful to invoke an operation with a CancellationToken other than CancellationToken.None, i.e.

new CancellationToken(canceled: true)

This support can be introduced by modifying ServerTestBuilder:

  1. Add a private field CancellationToken cancellationToken to the class, which by default is initialized to CancellationToken.None to preserve the current behavior
  2. Add a method IServerTestBuilder WithCancellationToken(CancellationToken cancellationToken) which sets the private field
  3. In ServerTestBuilder +124, send the request with the value of this.cancellationToken

What are your thoughts on this idea and its potential implementation? I'd be happy to implement it myself; your library has already been very helpful in testing my WebApi application.

@bgold09 bgold09 changed the title supporting sending request with CancellationToken support sending requests with CancellationToken Jul 27, 2016
@ivaylokenov ivaylokenov changed the title support sending requests with CancellationToken Support sending requests with CancellationToken Jul 27, 2016
@ivaylokenov
Copy link
Owner

@bgold09 I believe it will be useful. Will implement it and upload a new version of the library in a few hours.

@ivaylokenov
Copy link
Owner

ivaylokenov commented Jul 27, 2016

@bgold09 I implemented the methods and tried it out but all requests with CancellationToken set to true throw errors that tasks have been canceled. I could not find any proper solution after an hour research.

Do you have any other suggestions?

@bgold09
Copy link
Author

bgold09 commented Jul 27, 2016

@ivaylokenov if you add a custom message handler to the HttpConfiguration used to initialize the server, does the CancellationToken at least flow through to its SendAsync() method?

Does the exception come from the server or the client message invoker?

@ivaylokenov
Copy link
Owner

ivaylokenov commented Jul 27, 2016

@bgold09 I tried with both custom OWIN handler and message handler on the HttpConfiguration and it doest not reach it. The code throws errors on the https://github.com/ivaylokenov/MyTested.WebApi/blob/master/src/MyTested.WebApi/Builders/Servers/ServerTestBuilder.cs#L124 line so I expect the task to throw in the client. The request does not gets processed on the server.

@bgold09
Copy link
Author

bgold09 commented Jul 27, 2016

Is an OperationCanceledException being thrown? If so, I suspect that CancellationToken.ThrowIfCancellationRequested() is being called somewhere in the depths of System.Net.Http.dll.

@ivaylokenov
Copy link
Owner

@bgold09 Actually it is TaskCanceledException. Looking through this code:

https://www.symbolsource.org/Public/Metadata/NuGet/Project/HttpClient/0.5.0/Release/.NETFramework,Version%3Dv4.0/Microsoft.Net.Http/Microsoft.Net.Http/System/Net/Http/HttpClient.cs?ImageName=Microsoft.Net.Http

There are numerous checks for the cancellation token and I guess it is normal that we receive the exception. I guess it will not be possible to implement this feature? I am out of ideas.

@bgold09
Copy link
Author

bgold09 commented Jul 28, 2016

Hmm, let me do a bit of digging and get back to you. I agree though, it looks like this is less straightforward than I originally thought.

@bgold09
Copy link
Author

bgold09 commented Jul 28, 2016

@ivaylokenov I think I understand what's happening now, as I was misunderstanding part of how CancellationTokens are supposed to be used. We should instead be asking the user to pass a CancellationTokenSource which can be used to get an associated CancellationToken.

What we actually would want here is to create a method IServerTestBuilder WithCancellationTokenSource(CancellationTokenSource cancellationTokenSource) and then do

invoker.SendAsync(this.httpRequestMessage, this.cancellationTokenSource.Token).Result;

For example, the user could then create a new CancellationTokenSource which cancels the operation after a certain amount of time:

var tokenSource = new new CancellationTokenSource(TimeSpan.FromSeconds(30));
MyWebApi.Server().Working().WithCancellationTokenSource(tokenSource);

@ivaylokenov
Copy link
Owner

ivaylokenov commented Jul 29, 2016

@bgold09 Yes, we both misunderstood it obviously. Otherwise, with the CancelationTokenSource it works now and the token is passed through the server.

One final thought before I implement this feature - if the only parameter the developer can use is a TimeSpan value it will be difficult for him/her to determine the exact value to set so that a particular cancelation is tested. It is not very deterministic. Any thoughts?

@bgold09
Copy link
Author

bgold09 commented Jul 29, 2016

Fair point; in that case, the user could then initialize a default CancellationTokenSource, keep it in a local variable, pass it to WithCancellationTokenSource() and later call CancellationTokenSource.Cancel() to manually do it. That should provide more control.

We should include one such example in the documentation, as well as an example where the CancellationTokenSource is initialized with a TimeSpan value.

@ivaylokenov
Copy link
Owner

ivaylokenov commented Aug 1, 2016

@bgold09 Unfortunately, when I tried that it did not work as again TaskCancelledException was thrown on the same line of code. The token fires cancellation on the HttpClient task as well. When using debugger, I can see that the IsCancellationRequested becomes true after the Cancel call but the exception still occurs.

This is the Owin class I tried:

    public class CancellationStartup
    {
        public static CancellationTokenSource CancellationTokenSource = new CancellationTokenSource();

        public void Configuration(IAppBuilder app)
        {
            app.Use((context, next) =>
            {
                CancellationTokenSource.Cancel();

                if (context.Request.CallCancelled.IsCancellationRequested)
                {
                    context.Response.StatusCode = 500;
                    context.Response.Write("Canceled");
                }

                return next();
            });
        }
    }

And this is the test:

MyWebApi
      .Server()
      .Working<CancellationStartup>()
      .WithCancellationTokenSource(CancellationStartup.CancellationTokenSource)
      .WithHttpRequestMessage(req => req.WithMethod(HttpMethod.Get))
      .ShouldReturnHttpResponseMessage()
      .WithStatusCode(HttpStatusCode.InternalServerError);

This feature became harder than it looked at first.

@bgold09
Copy link
Author

bgold09 commented Aug 1, 2016

Is there any way to swallow the exception being thrown in the HttpClient? If I understood correctly, the cancellation does happen in the startup code you have above.

The interesting case for me is whether or not I can get this cancellation notification in some custom message handler, so that I can ensure that it is properly handling this case from the perspective of my service.

@ivaylokenov
Copy link
Owner

ivaylokenov commented Aug 2, 2016

@bgold09 Yes, I am canceling the token in the OWIN inline handler, and then the HttpClient throws exception because it has subscribed to the same token. I am still researching whether something can be done to catch it correctly. When I think about it, however, if I cancel an operation, the server should not return any HttpResponseMessage. For example, if the client has disconnected, the server should cancel all tasks in order to save resources, but it does not need to return any response, because there is not client to receive it anymore.

Otherwise - for a particular handler, which handles the cancellation - will a simple unit test work for validating the behavior without running the whole pipeline? Can I see the handler code?

@bgold09
Copy link
Author

bgold09 commented Aug 4, 2016

I agree, I think it's fine to simply unit test this.

Originally I was trying to see if the cancellation could be piped through to various methods as well, but I think now that I better understand cancellation tokens this is not really needed.

Feel free to close this and thanks for following up.

@ivaylokenov
Copy link
Owner

You are welcome! 👍

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

No branches or pull requests

2 participants