Make upload more robust by ignoring spurious errors while polling the scan status.

snapcraft/storeapi/_upload.py

@@ -20,13 +20,13 @@
 import time
 import os
 
+import requests
 from concurrent.futures import ThreadPoolExecutor
 from progressbar import (ProgressBar, Percentage, Bar, AnimatedMarker)
 from requests_toolbelt import (MultipartEncoder, MultipartEncoderMonitor)
 
 from .common import (
     get_oauth_session,
-    is_scan_completed,
     retry,
 )
 from .compat import open, quote_plus, urljoin
@@ -318,6 +318,30 @@ def get_post_files(metadata=None):
     return files
 
 
+def is_scan_completed(response):
+    """Return True if the response indicates the scan process completed."""
+    if response is None:
+        # To cope with spurious connection failures lacking a proper response:
+        # either we'll retry and succeed or we fail for all retries and report
+        # an error.
+        return False
+    if response.ok:
+        return response.json().get('completed', False)
+    return False
+
+
+def get_scan_status(session, url):
+    try:
+        resp = session.get(url)
+        return resp
+    except (requests.ConnectionError, requests.HTTPError):
+        # Something went wrong and we couldn't acquire the status. Upper
+        # level (is_scan_completed) will deal with the None response
+        # meaning we don't know the status. This avoid a spurious
+        # connection error breaking an upload for a wrong reason.
+        return None
+
+
 def get_scan_data(session, status_url):
     """Return metadata about the state of the upload scan process."""
     # initial retry after 5 seconds
@@ -328,7 +352,7 @@ def get_scan_data(session, status_url):
            delay=SCAN_STATUS_POLL_DELAY,
            backoff=1)
     def get_status():
-        return session.get(status_url)
+        return get_scan_status(session, status_url)
 
     response, aborted = get_status()
 

snapcraft/storeapi/common.py

@@ -67,13 +67,6 @@ def store_api_call(path, session=None, method='GET', data=None):
     return result
 
 
-def is_scan_completed(response):
-    """Return True if the response indicates the scan process completed."""
-    if response.ok:
-        return response.json().get('completed', False)
-    return False
-
-
 def retry(terminator=None, retries=3, delay=3, backoff=2, logger=None):
     """Decorate a function to automatically retry calling it on failure.
 

snapcraft/tests/test_storeapi_upload.py

@@ -17,13 +17,20 @@
 import json
 import os
 import tempfile
+import unittest
 
 from mock import ANY, call, patch
-from requests import Response
+from requests import (
+    ConnectionError,
+    HTTPError,
+    Response,
+)
 
 from snapcraft import tests
 from snapcraft.storeapi._upload import (
     get_upload_url,
+    get_scan_status,
+    is_scan_completed,
     upload_app,
     upload_files,
     upload,
@@ -650,3 +657,28 @@ def test_get_upload_url(self):
             upload_url = "http://example.com/click-package-upload/app.dev/"
             url = get_upload_url('app.dev')
             self.assertEqual(url, upload_url)
+
+
+class FakeSession(object):
+
+    def __init__(self, exc):
+        self.exc = exc
+
+    def get(self, url):
+        raise self.exc()
+
+
+class ScanStatusTestCase(unittest.TestCase):
+
+    def test_is_scan_complete_for_none(self):
+        self.assertFalse(is_scan_completed(None))
+
+    def get_scan_status(self, exc):
+        raiser = FakeSession(exc)
+        return get_scan_status(raiser, 'foo')
+
+    def test_get_status_connection_error(self):
+        self.assertIsNone(self.get_scan_status(ConnectionError))
+
+    def test_get_status_http_error(self):
+        self.assertIsNone(self.get_scan_status(HTTPError))