Issue 4767 - Improve error reporting in update_update_manifests

sitescripts/extensions/utils.py

Index: sitescripts/extensions/utils.py
===================================================================
--- a/sitescripts/extensions/utils.py
+++ b/sitescripts/extensions/utils.py
@@ -26,6 +26,7 @@
 from ConfigParser import SafeConfigParser, NoOptionError
 from StringIO import StringIO
 from sitescripts.utils import get_config
+from xml.parsers.expat import ExpatError
 
 PACKAGE_SUFFIXES = {
     'gecko': '.xpi',
@@ -250,17 +251,31 @@
         try:
             return urllib.urlopen(url)
         except IOError as e:
-            error = e
+            error = Exception('Error {0} while opening {1} url'
+                              .format(e, url))
             time.sleep(5)
     raise error
 
 
+def _parseXMLDocument(url, attempts=2):
+    for i in range(attempts):
+        page = _urlopen(url)
+        content = page.read()
+        page.close()

[Sebastian Noack, 2017/01/17 09:33:32]

The close() should have gone into a finally block, or even better using the with
statement:

  with contextlib.closing(_urlopen(url)) as page:
      content = page.read()

[Sebastian Noack, 2017/01/17 10:16:56]

Yes, in Python 2, the repsonse object returned by urlopen() doesn't implement
__enter__/__exit__. So you either have to use the contextlib.closing() decorator
before passing it to the with statement or use a traditionally try-finally
block. I suggest doing the former, and removing the decorator once we migrate to
Python 3.

Also, please reply inline to inline comment.

[Sebastian Noack, 2017/01/17 10:20:15]

Well, alternatively you could use try-finally. But IMO, the with-statement is
nicer. For reference, here is how we did it in jsHydra where we support both
Python 2+3: https://hg.adblockplus.org/jshydra/file/tip/abp_rewrite.py#l11

[Vasily Kuznetsov, 2017/01/17 10:30:34]

Yeah, with is nicer, especially having Python 3 in mind.

+        try:
+            return dom.parseString(content)
+        except ExpatError as err:
+            exception = Exception('Error {0} while parsing xml:\n{1}\nfrom {2}'
+                                  .format(err, content, url))
+    raise exception
+
+
 def _getMozillaDownloadLink(galleryID):
     """
     gets download link for a Gecko add-on from the Mozilla Addons site
     """
     url = 'https://services.addons.mozilla.org/en-US/firefox/api/1/addon/%s' % _urlencode(galleryID)
-    document = dom.parse(_urlopen(url))
+    document = _parseXMLDocument(url)
     linkTags = document.getElementsByTagName('install')
     linkTag = linkTags[0] if len(linkTags) > 0 else None
     versionTags = document.getElementsByTagName('version')