2

i have the following function will will get me the html source of some website over a proxy, its working fine except some times when server returns 503(server unavailable) or any other exception it never goes into the catch statement.

in the catch statement , the function is supposed calls itself recursively, up to 4 times, if the request keeps failing after 4 tries then null is returned.

private static string GetPageHTML(string link,bool useprx)
            {
                int tryCount = 0;
                WebClient client = new WebClient() { Proxy = new WebProxy(ProxyManager.GetProxy()) { Credentials = new NetworkCredential("xx", "xx") } };

                try
                {
                return client.DownloadString(link);
                }
                catch (WebException ex)
                {
                    var statuscode = ((HttpWebResponse)ex.Response).StatusCode;
                    {

                        if (tryCount == 3)
                        {
                            return null;
                        }

                        switch (statuscode)
                        {
                            case (HttpStatusCode.Forbidden):
                                tryCount++;
                                System.Threading.Thread.Sleep(5000); 
                                return GetPageHTML(link, useprx); 

                            case (HttpStatusCode.NotFound): 
                                return null; 


                            case (HttpStatusCode.GatewayTimeout):
                                tryCount++;
                                System.Threading.Thread.Sleep(5000); 
                                return GetPageHTML(link, useprx); 


                            case (HttpStatusCode.ServiceUnavailable) :
                                 tryCount++;
                                System.Threading.Thread.Sleep(5000); 
                                return GetPageHTML(link, useprx);

                            default: return null;

                        }
                    }
                }
            }

so why it never goes into the catch statement?

user1590636
  • 1,174
  • 6
  • 26
  • 56
  • 1
    Putting the thread to sleep for 5 seconds is wasting a thread. If you want to wait, set a timer or something. – Anthony Pegram Mar 28 '13 at 05:38
  • 2
    Also, your `tryCount` is a local. When you recursively call the method, *it gets a different local.* If your recursive method call fails, it will call itself, again with a new local. I think you will find that observing a tryCount value of 3 should be *difficult.* You probably want to refactor and either pass in the count as a parameter so you can keep track of it, or preferably refactor the potential failing code out further so that you can isolate it and keep track of it outside of that failure point. – Anthony Pegram Mar 28 '13 at 05:40
  • @AnthonyPegram thats right, that needs to be fixed too – user1590636 Mar 28 '13 at 05:44

1 Answers1

3

It's probably returning an exception that is not of type WebException. To catch all exceptions under the sun you have to include "catch Exception" as a fallback

Add the fall back catch, after the WebException catch, and debug it to see what type of exception it's really returning

TGH
  • 38,769
  • 12
  • 102
  • 135
  • +1 but note that catching all exceptions is generally a **bad practice** in production code. You should only catch exceptions you can meaningfully handle. – p.s.w.g Mar 28 '13 at 05:40
  • @p.s.w.g yes, i need to handle all the above. – user1590636 Mar 28 '13 at 05:50
  • @p.s.w.g, what's the alternative? – JGood Nov 16 '13 at 17:33
  • @goodwince As I said, only catch exceptions you can meaningfully handle, and let everything else bubble up. Usually you'll define a top-level global exception handler to deal with any uncaught exceptions for the purpose of logging / error reporting. See also http://programmers.stackexchange.com/questions/164256/is-catching-general-exceptions-really-a-bad-thing – p.s.w.g Nov 16 '13 at 17:41
  • Excellent link. The linked discussion describes it as situational, but aim for bubbling it up. Thanks for clarifying, I wanted to make sure it was still being globally handled. – JGood Nov 16 '13 at 21:21