Issue 4767 - Improve error reporting in update_update_manifests

sitescripts/extensions/utils.py

 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-2016 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
@@ skipping 233 matching lines @@
 
 
 def _urlopen(url, attempts=3):
     """
     Tries to open a particular URL, retries on failure.
     """
     for i in range(attempts):
         try:
             return urllib.urlopen(url)
         except IOError as e:
-            error = e

[Vasily Kuznetsov, 2017/01/13 18:12:11]

It would probably be good to improve the error reporting here as well. Otherwise
`urllib.urlopen` throws exceptions like this:

IOError: [Errno socket error] [Errno 8] nodename nor servname provided, or not
known

and that is also not superhelpful (e.g. it doesn't have the URL).

+            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):
     """
@@ skipping 119 matching lines @@
     if not extensions:
         return
 
     updates = {}
     for extension in extensions:
         updates[extension['basename']] = {
             'url': extension['updateURL'],
             'version': extension['version']
         }
     writeLibabpUpdateManifest(path, updates)