tags:

views:

671

answers:

5

I would like to know the proper way to catch a 404 error with c# asp.net here is the code I'm using

HttpWebRequest request = (HttpWebRequest) WebRequest.Create(String.Format("http://www.gravatar.com/avatar/{0}?d=404", hashe));

// execute the request
try
{
    //TODO: test for good connectivity first
    //So it will not update the whole database with bad avatars
    HttpWebResponse response = (HttpWebResponse)request.GetResponse();
    Response.Write("has avatar");
}
catch (Exception ex)
{
    if (ex.ToString().Contains("404"))
    {
         Response.Write("No avatar");
    }
}

This code works but I just would like to know if this is the most efficient.

A: 

StatusCode property is what I think, you are looking for.

shahkalpesh
+1  A: 

See at MSDN about status of the response:

...
catch(WebException e) {
  Console.WriteLine("The following error occured : {0}",e.Status);  
}
...
Dror
-1: the WebResponse needs to be in a `using` block
John Saunders
@John Saunders - I'll be more than happy to pass it along to MSDN (where I copied the sample from...).The purpose of this code is to show the usage of StatusCode, not be as efficient as possible.
Dror
A `using` block isn't about efficiency. The `IDisposable` pattern is a basic pattern of .NET. I see the pattern ignored in production code, all the time. The only solution is to use it all the time, even in samples. This is especially true because samples are copied (as you did). I'd let you off if there was a comment saying "bad practice - in real life, use `using`".
John Saunders
@John Saunders - I left only the part I wanted to show, Just for you :-)
Dror
@Dror: downvote removed
John Saunders
+4  A: 

I haven't tested this, but it should work

try
{
    // TODO: Make request.
}
catch (WebException ex)
{
    if (ex.Status == WebExceptionStatus.ProtocolError) {
        HttpWebResponse resp = ex.Response as HttpWebResponse;
        if (resp != null && resp.StatusCode == HttpStatusCode.NotFound)
        {
            // TODO: Handle 404 error.
        }
        else
            throw;
    }
    else
        throw;
}
MiffTheFox
-1: the WebResponse from `GetResponse` needs to be in `using` block
John Saunders
@John Saunders - I was adapting the OP's code, not optimizing it.
MiffTheFox
@MiffTheFox: `using` blocks are not an optimization - they're pretty basic - except to the person who copies and pastes your example.
John Saunders
@John - And maybe I was only expecting them to copy/paste the `catch` block, seeing as I had the exact same code in the try as the OP. You should really be downvoaing this question altogether because of the OP's code then.
MiffTheFox
@MiffTheFox: It makes no sense to downvote you because of the OP
John Saunders
@John we forget here is sample code. This is case it is another way to 404, not how to use GetResponse. -1 seems a little harsh. +1 to Miff for answering the question.
David Basarab
@David: I don't forget - I see a large amount of sample code cut and pasted as-is. I'm trying to encourage those who answer questions to answer them in such a way that no problems would be caused by copy and paste. Note I've removed downvotes where they have been corrected.
John Saunders
@John I think it is good you point it out in a comment. The way I look at down voting is if the code given doesn't solve the problem. Thank you for removing the down vote.
David Basarab
@John - Fine, I got rid of everything but the catch, happy now?
MiffTheFox
@MiffTheFox: downvote removed. Thanks for playing.
John Saunders
A: 

Catch the proper exception type WebException:

try
{
    var request = (HttpWebRequest) WebRequest.Create(String.Format("http://www.gravatar.com/avatar/{0}?d=404", hashe));

    using(var response = (HttpWebResponse)request.GetResponse())
        Response.Write("has avatar");
}
catch(WebException e) 
{
  if(e.Response.StatusCode == 404) 
    Response.Write("No avatar");
}
Nick Craver
-1: the WebResponse needs to be in a `using` block
John Saunders
@John Saunders I don't debate you there, but that wasn't the question, he asked the best way to capture a 404. My changes to his code were limited to answering the question, to make the change as simple and obvious as possible.
Nick Craver
@Nick: I will always downvote example code that can lead to resource leaks. It's not hard to propertly handle `IDisposable` - just do it.
John Saunders
@Nick: downvote removed
John Saunders
@John Saunders: Fixed, I suppose "if this is the most efficient" makes it apply to the question.
Nick Craver
+11  A: 
try
{
    var request = WebRequest.Create(uri);
    using (var response = request.GetResponse())
    {
        using (var responseStream = response.GetResponseStream())
        {
            // Process the stream
        }
    }
}
catch (WebException ex)
{
    if (ex.Status == WebExceptionStatus.ProtocolError &&
        ex.Response != null)
    {
        var resp = (HttpWebResponse) ex.Response;
        if (resp.StatusCode == HttpStatusCode.NotFound)
        {
            // Do something
        }
        else
        {
            // Do something else
        }
    }
    else
    {
        // Do something else
    }
}
John Saunders
lol @ being the IDisposable police and giving everyone a -1 for not wrapping the response in a using block.
Rich
It's a tough job, but _someone_ has to do it. OTOH, I almost didn't add this answer, since it might seem I was dinging everyone else to make mine the top-rated answer.
John Saunders
@Downvoter: care to say why? I did.
John Saunders
I actually upvoted, but I just noticed one thing: There should probably be a `throw` (rethrow) at the end of your `catch`, otherwise this will just silently eat any other type of `WebException`.
Aaronaught
@John Saunders: Why don't you have a using around your request as well?
Joel Etherton
@Joel: `WebRequest` doesn't implement `IDisposable`.
John Saunders
@Aaronaught: There should be a couple of "else do something else"
John Saunders
@John Saunders: Alrighty. Good to know. I'll have to look this up/read more about this IDisposable concept/using etc. It's a topic currently going around in our office. Thanks.
Joel Etherton