4

I have written a method which get ETag from eq. XML file on server. Am I correct wrote abort task if timeout (GetResponseAsync() doesn't have CancellationToken) and I have no other idea how to do Exception.

Here's code:

    public static async Task<string> GetETagAsync(Uri feedLink)
    {
        const int millisecondsTimeout = 2500;
        WebRequest webRequest = WebRequest.Create(feedLink);
        webRequest.Method = "HEAD";
        try
        {
            Task<WebResponse> webResponse = webRequest.GetResponseAsync();
            if (await Task.WhenAny(webResponse, Task.Delay(millisecondsTimeout)) == webResponse)
            {
                using (var result = webResponse.Result)
                {
                    return result.Headers["ETag"];
                }
            }
            else
            {
                webRequest.Abort();
                return null;
            }
        }
        catch (Exception)
        {
            return null;
        }
    }

Edit

I have made some changes. Rewrite exceptions and use class from this topic: GetResponseAsync does not accept cancellationToken

Code:

    public static async Task<string> GetETagAsync(Uri feedLink)
    {
        const int millisecondsTimeout = 2500;
        var cancellationTokenSource = new CancellationTokenSource();
        WebRequest webRequest = WebRequest.Create(feedLink);
        webRequest.Method = "HEAD";

        try
        {
            Task<WebResponse> webResponse = WebRequestExtensions.GetResponseAsync(webRequest, cancellationTokenSource.Token);
            if (await Task.WhenAny(webResponse, Task.Delay(millisecondsTimeout)) == webResponse)
            {
                using (var result = webResponse.Result)
                {
                    return result.Headers["ETag"];
                }
            }
            else
            {
                cancellationTokenSource.Cancel();
                return null;
            }
        }
        catch (AggregateException ex)
        {
            if (ex.InnerException is WebException)
                return null;

            throw;
        }
    }

public static class WebRequestExtensions
{
    public static async Task<WebResponse> GetResponseAsync(this WebRequest request, CancellationToken cancellationToken)
    {
        using (cancellationToken.Register(() => request.Abort(), useSynchronizationContext: false))
        {
            try
            {
                var response = await request.GetResponseAsync();
                cancellationToken.ThrowIfCancellationRequested();
                return (WebResponse)response;
            }
            catch (WebException ex)
            {
                if (ex.Status == WebExceptionStatus.RequestCanceled)
                {
                    cancellationToken.ThrowIfCancellationRequested();
                }

                if (cancellationToken.IsCancellationRequested)
                {
                    throw new TaskCanceledException(ex.Message, ex);
                }

                throw;
            }
        }
    }
}

Is it correct now?

Community
  • 1
  • 1
rechandler
  • 756
  • 8
  • 22
  • 2
    You almost certainly don't want to be catching all exceptions and returning `null`. If you can't handle the exception, just let it propagate out. – Servy Apr 01 '14 at 20:29
  • possible duplicate of [GetResponseAsync does not accept cancellationToken](http://stackoverflow.com/questions/19211972/getresponseasync-does-not-accept-cancellationtoken) – noseratio Apr 01 '14 at 20:38
  • @Noseratio I read this, but I use `webRequest.Abort()` instead of your method and I want to know is it correct – rechandler Apr 01 '14 at 20:43
  • @Servy I understand and agree but it's just method which can minimize data transfer. And actually only exception (and only once) I get was from site http://niebezpiecznik.pl there was something that "server refuse connection(?)" I don't remember what was exactly. – rechandler Apr 01 '14 at 20:54
  • 2
    @J.Marciniak That doesn't mean you should just swallow the exceptions and pretend they don't exist. When there's a problem you want to *know about it* rather than having your code break further down the line without being able to understand why. – Servy Apr 01 '14 at 21:07
  • 1
    @J.Marciniak, did you not notice `request.Abort()` in [the code there](http://stackoverflow.com/a/19215782/1768303)? You can call `request.Abort()` from outside, too. – noseratio Apr 01 '14 at 21:23
  • 1
    @Noseratio you're right, sorry for my mistake. – rechandler Apr 02 '14 at 09:51
  • @Servy edit first post ;) – rechandler Apr 02 '14 at 11:18
  • @J.Marciniak I was dealing with this in a polling service, and what you have is essentially what I have, but in a loop, with one critical difference. That difference is that I abort the request every time. I have not copy/pasted your code to test it, but it should work for the first attempt, and the second, assuming it's within some window - maybe the default (non-async) `GetResponse()` of 100000ms? But eventually your `Task.WhenAny` will start returning the `Task.Delay` every time. Right now I don't know exactly why it fails on successive attempts. – Eric Lease Jan 27 '16 at 15:05
  • @J.Marciniak I think it has to do with the async response stream left pending. If you are not going to read the response stream until the end, then you should call `request.Abort()` regardless of whether the task completed before the `Task.Delay` or not. If you do not abort (or read until the end of the stream maybe, haven't tried this) future `Tasks` involving `GetResponseAsync()` will sit in a waiting status until after the `Task.Delay` completes. Yes, the extension method will call `Abort()`, but I think you want to also call it after reading the critical data from the response. – Eric Lease Jan 27 '16 at 15:07

2 Answers2

4

You can use the WebRequestExtensions class I wrote here to resolve the problem: https://github.com/openstacknetsdk/openstack.net/blob/master/src/corelib/Core/WebRequestExtensions.cs

This class fully supports both a CancellationToken and the WebRequest.Timeout property.

Sam Harwell
  • 97,721
  • 20
  • 209
  • 280
  • I read this: but not sure is it correct [Cancel an async webrequest?](http://stackoverflow.com/questions/8635723/cancel-an-async-webrequest) – rechandler Apr 01 '14 at 20:42
  • @J.Marciniak I'm not sure how that relates to my answer? – Sam Harwell Apr 01 '14 at 20:49
  • it seems `GetResponseAsync(this WebRequest request)` has been added to the MVC code, but not `GetResponseAsync(this WebRequest request, CancellationToken cancellationToken)` THANK YOU – Brad Dec 02 '14 at 16:41
1

Since the method doesn't natively support any means of stopping the task, it can't be stopped. There is nothing that you can do that will prevent it from continuing to do whatever it plans to do. What you can do is ensure that this async method's task doesn't wait for it to finish. While you can just return a value, as you're doing, this seems to represent an exceptional case; you should probably be throwing an exception to indicate that the operation timed out.

Servy
  • 202,030
  • 26
  • 332
  • 449