c# - is it ok to embed a "try/catch" within a "using" statement for a web request? Is my code correct?

Question

Is it ok to embed a "try/catch" within a "using" statement for a web request? Is my code correct? That is my requirements are:

1. Want to use the "using" statement to make sure resources are released in any case for HttpWebResponse

   • But still want to do some custom stuff if there is an exception re HttpWebResponse and "response = (HttpWebResponse)request.GetResponse();" in particular.

My source code:

        var result = new HttpHeaderInfo();
        HttpWebRequest request = null;
        HttpWebResponse response = null;
        using (response)
        {
            try
            {
                request = (HttpWebRequest)WebRequest.Create(uri);
                request.Method = "HEAD";
                request.KeepAlive = false;
                request.Timeout = Properties.Settings.Default.WebTimeoutDefault;

                response = (HttpWebResponse)request.GetResponse();
                result.LastModified = response.LastModified;
                result.ContentType = response.ContentType;
                result.StatusCode = response.StatusCode;
                result.ContentLength = response.ContentLength;
            }
            catch (Exception ex)
            {
                if (ex is InvalidOperationException ||
                    ex is ProtocolViolationException ||
                    ex is WebException)
                {
                    result.HttpError = ex;
                    result.LastModified = System.DateTime.MinValue;
                    result.ContentType = null;
                }
                else { throw; }
            }

        }

thanks

Answer 1

It is OK, but a little redundant; in a general sense, you could easily remove the using block, add a finally block after the catch, and explicitly call Dispose in there, which would reduce the nesting in your code.

In a more specific sense, what bugs me a little is that you don't actually assign response until you get inside the using block, and the explicit variable declarations are unnecessary and confusing in this context. I would rewrite it as:

HttpHeaderInfo result;
try
{
    var request = (HttpWebRequest)WebRequest.Create(uri);
    request.Method = "HEAD";
    request.KeepAlive = false;
    request.Timeout = Properties.Settings.Default.WebTimeoutDefault;

    using (HttpWebResponse response = (HttpWebResponse)request.GetResponse())
    {
        result = new HttpHeaderInfo();
        result.LastModified = response.LastModified;
        result.ContentType = response.ContentType;
        result.StatusCode = response.StatusCode;
        result.ContentLength = response.ContentLength;
    }
}
catch (WebException ex)
{
    // etc.
}

This is a lot clearer than the original form. Also note that I'm catching WebException, not the generic System.Exception. You should catch specific exception types instead of catching generic exceptions and then checking their type.