Rietveld: refactor and improve retries with UrlError exception.

rietveld.py

 # coding: utf-8
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 """Defines class Rietveld to easily access a rietveld instance.
 
 Security implications:
 
 The following hypothesis are made:
 - Rietveld enforces:
@@ skipping 413 matching lines @@
           if retry >= (self._maxtries - 1):
             raise
           flake_codes = [500, 502, 503]
           if retry_on_404:
             flake_codes.append(404)
           if e.code not in flake_codes:
             raise
         except urllib2.URLError, e:
           if retry >= (self._maxtries - 1):
             raise
-          if (not 'Name or service not known' in e.reason and
-              not 'EOF occurred in violation of protocol' in e.reason and
-              not 'timed out' in e.reason and
-              not 'The handshake operation timed out' in e.reason and
-              # On windows we hit weird bug http://crbug.com/537417
-              # with message '[Errno 10060] A connection attempt failed...'
-              not (sys.platform.startswith('win') and
-                   isinstance(e.reason, socket.error) and
-                   e.reason.errno == errno.ETIMEDOUT
-                )
-              ):
-            # Usually internal GAE flakiness.
+
+          def is_transient():
+            # The idea here is to retry if the error isn't permanent.
+            # Unfortunately, there are so many different possible errors,
+            # that we end up enumerating those that are known to us to be
+            # transient.
+            # The reason can be a string or another exception, e.g.,
+            # socket.error or whatever else.
+            reason_as_str = str(e.reason)
+            for retry_anyway in [

[Sergiy Byelozyorov, 2015/12/03 14:54:45]

return any(str in reason_as_str for str in [...]):

[tandrii(chromium), 2015/12/03 14:58:27]

I found "str in" for "str in" confusing, even though I love list comprehensions,
so will leave as is.

+                'Name or service not known',
+                'EOF occurred in violation of protocol',
+                'The handshake operation timed out',

[Adrian Kuegel, 2015/12/03 14:53:31]

Drive-by: this line seems to be redundant.

[tandrii(chromium), 2015/12/03 14:58:27]

Totally!

+                'timed out']:
+              if retry_anyway in reason_as_str:
+                return True
+            return False  # Assume permanent otherwise.
+          if not is_transient():
             raise
         except socket.error, e:
           if retry >= (self._maxtries - 1):
             raise
           if not 'timed out' in str(e):
             raise
         # If reaching this line, loop again. Uses a small backoff.
         time.sleep(min(10, 1+retry*2))
     except urllib2.HTTPError as e:
       print 'Request to %s failed: %s' % (e.geturl(), e.read())
@@ skipping 276 matching lines @@
       self, issue, patchset, reason, clobber, revision, builders_and_tests,
       master=None, category='cq'):
     logging.info('ReadOnlyRietveld: triggering try jobs %r for issue %d' %
         (builders_and_tests, issue))
 
   def trigger_distributed_try_jobs(  # pylint:disable=R0201
       self, issue, patchset, reason, clobber, revision, masters,
       category='cq'):
     logging.info('ReadOnlyRietveld: triggering try jobs %r for issue %d' %
         (masters, issue))