Launchpad itself

Merge lp:~cjwatson/launchpad/snap-store-upload-error-debug into lp:launchpad

  1. snap-store-upload-error-debug
  2. Merge into devel
Proposed by Colin Watson
Status: Merged
Merged at revision: 18358
Proposed branch: lp:~cjwatson/launchpad/snap-store-upload-error-debug
Merge into: lp:launchpad
Diff against target: 278 lines (+108/-46)
6 files modified
lib/lp/snappy/interfaces/snapbuildjob.py (+3/-0)
lib/lp/snappy/interfaces/snapstoreclient.py (+51/-42)
lib/lp/snappy/model/snapbuildjob.py (+19/-1)
lib/lp/snappy/model/snapstoreclient.py (+11/-2)
lib/lp/snappy/tests/test_snapbuildjob.py (+4/-1)
lib/lp/snappy/tests/test_snapstoreclient.py (+20/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/snap-store-upload-error-debug
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+322715@code.launchpad.net

Commit message

Include the full response from unscanned-upload as an error_detail variable in BadUploadResponse OOPSes.

Description of the change

I'm hoping to get something I can use to figure out bug 1683819.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) wrote :

Let's just hope it doesn't echo back the whole blob.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/snappy/interfaces/snapbuildjob.py'
--- lib/lp/snappy/interfaces/snapbuildjob.py 2017-04-03 14:43:22 +0000
+++ lib/lp/snappy/interfaces/snapbuildjob.py 2017-04-18 21:59:36 +0000
@@ -57,6 +57,9 @@
57 error_message = TextLine(57 error_message = TextLine(
58 title=_("Error message"), required=False, readonly=True)58 title=_("Error message"), required=False, readonly=True)
5959
60 error_detail = TextLine(
61 title=_("Error message detail"), required=False, readonly=True)
62
60 store_url = TextLine(63 store_url = TextLine(
61 title=_("The URL on the store corresponding to this build"),64 title=_("The URL on the store corresponding to this build"),
62 required=False, readonly=True)65 required=False, readonly=True)
6366
=== modified file 'lib/lp/snappy/interfaces/snapstoreclient.py'
--- lib/lp/snappy/interfaces/snapstoreclient.py 2016-12-08 17:39:19 +0000
+++ lib/lp/snappy/interfaces/snapstoreclient.py 2017-04-18 21:59:36 +0000
@@ -1,4 +1,4 @@
1# Copyright 2016 Canonical Ltd. This software is licensed under the1# Copyright 2016-2017 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Interface for communication with the snap store."""4"""Interface for communication with the snap store."""
@@ -17,6 +17,7 @@
17 'NeedsRefreshResponse',17 'NeedsRefreshResponse',
18 'ReleaseFailedResponse',18 'ReleaseFailedResponse',
19 'ScanFailedResponse',19 'ScanFailedResponse',
20 'SnapStoreError',
20 'UnauthorizedUploadResponse',21 'UnauthorizedUploadResponse',
21 'UploadNotScannedYetResponse',22 'UploadNotScannedYetResponse',
22 ]23 ]
@@ -27,48 +28,56 @@
27from zope.interface import Interface28from zope.interface import Interface
2829
2930
31class SnapStoreError(Exception):
32
33 def __init__(self, message="", detail=None):
34 super(SnapStoreError, self).__init__(message)
35 self.message = message
36 self.detail = detail
37
38
30@error_status(httplib.INTERNAL_SERVER_ERROR)39@error_status(httplib.INTERNAL_SERVER_ERROR)
31class BadRequestPackageUploadResponse(Exception):40class BadRequestPackageUploadResponse(SnapStoreError):
32 pass41 pass
3342
3443
35class BadUploadResponse(Exception):44class BadUploadResponse(SnapStoreError):
36 pass45 pass
3746
3847
39class BadRefreshResponse(Exception):48class BadRefreshResponse(SnapStoreError):
40 pass49 pass
4150
4251
43class NeedsRefreshResponse(Exception):52class NeedsRefreshResponse(SnapStoreError):
44 pass53 pass
4554
4655
47class UnauthorizedUploadResponse(Exception):56class UnauthorizedUploadResponse(SnapStoreError):
48 pass57 pass
4958
5059
51class BadScanStatusResponse(Exception):60class BadScanStatusResponse(SnapStoreError):
52 pass61 pass
5362
5463
55class UploadNotScannedYetResponse(Exception):64class UploadNotScannedYetResponse(SnapStoreError):
56 pass65 pass
5766
5867
59class ScanFailedResponse(Exception):68class ScanFailedResponse(SnapStoreError):
60 pass69 pass
6170
6271
63class BadSearchResponse(Exception):72class BadSearchResponse(SnapStoreError):
64 pass73 pass
6574
6675
67class BadReleaseResponse(Exception):76class BadReleaseResponse(SnapStoreError):
68 pass77 pass
6978
7079
71class ReleaseFailedResponse(Exception):80class ReleaseFailedResponse(SnapStoreError):
72 pass81 pass
7382
7483
7584
=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py 2017-04-03 14:43:22 +0000
+++ lib/lp/snappy/model/snapbuildjob.py 2017-04-18 21:59:36 +0000
@@ -61,6 +61,7 @@
61 ISnapStoreClient,61 ISnapStoreClient,
62 ReleaseFailedResponse,62 ReleaseFailedResponse,
63 ScanFailedResponse,63 ScanFailedResponse,
64 SnapStoreError,
64 UnauthorizedUploadResponse,65 UnauthorizedUploadResponse,
65 UploadNotScannedYetResponse,66 UploadNotScannedYetResponse,
66 )67 )
@@ -165,7 +166,7 @@
165 return oops_vars166 return oops_vars
166167
167168
168class ManualReview(Exception):169class ManualReview(SnapStoreError):
169 pass170 pass
170171
171172
@@ -216,6 +217,16 @@
216 self.metadata["error_message"] = message217 self.metadata["error_message"] = message
217218
218 @property219 @property
220 def error_detail(self):
221 """See `ISnapStoreUploadJob`."""
222 return self.metadata.get("error_detail")
223
224 @error_detail.setter
225 def error_detail(self, detail):
226 """See `ISnapStoreUploadJob`."""
227 self.metadata["error_detail"] = detail
228
229 @property
219 def store_url(self):230 def store_url(self):
220 """See `ISnapStoreUploadJob`."""231 """See `ISnapStoreUploadJob`."""
221 return self.metadata.get("store_url")232 return self.metadata.get("store_url")
@@ -262,6 +273,12 @@
262 def resume(self, *args, **kwargs):273 def resume(self, *args, **kwargs):
263 self._do_lifecycle(self.job.resume, *args, **kwargs)274 self._do_lifecycle(self.job.resume, *args, **kwargs)
264275
276 def getOopsVars(self):
277 """See `IRunnableJob`."""
278 oops_vars = super(SnapStoreUploadJob, self).getOopsVars()
279 oops_vars.append(('error_detail', self.error_detail))
280 return oops_vars
281
265 def run(self):282 def run(self):
266 """See `IRunnableJob`."""283 """See `IRunnableJob`."""
267 client = getUtility(ISnapStoreClient)284 client = getUtility(ISnapStoreClient)
@@ -282,6 +299,7 @@
282 raise299 raise
283 except Exception as e:300 except Exception as e:
284 self.error_message = str(e)301 self.error_message = str(e)
302 self.error_detail = getattr(e, "detail", None)
285 if isinstance(e, UnauthorizedUploadResponse):303 if isinstance(e, UnauthorizedUploadResponse):
286 mailer = SnapBuildMailer.forUnauthorizedUpload(self.snapbuild)304 mailer = SnapBuildMailer.forUnauthorizedUpload(self.snapbuild)
287 mailer.sendAll()305 mailer.sendAll()
288306
=== modified file 'lib/lp/snappy/model/snapstoreclient.py'
--- lib/lp/snappy/model/snapstoreclient.py 2017-02-27 12:01:08 +0000
+++ lib/lp/snappy/model/snapstoreclient.py 2017-04-18 21:59:36 +0000
@@ -179,13 +179,22 @@
179 try:179 try:
180 response = urlfetch(180 response = urlfetch(
181 unscanned_upload_url, method="POST", data=encoder,181 unscanned_upload_url, method="POST", data=encoder,
182 headers={"Content-Type": encoder.content_type})182 headers={
183 "Content-Type": encoder.content_type,
184 "Accept": "application/json",
185 })
183 response_data = response.json()186 response_data = response.json()
184 if not response_data.get("successful", False):187 if not response_data.get("successful", False):
185 raise BadUploadResponse(response.text)188 raise BadUploadResponse(response.text)
186 return {"upload_id": response_data["upload_id"]}189 return {"upload_id": response_data["upload_id"]}
187 except requests.HTTPError as e:190 except requests.HTTPError as e:
188 raise BadUploadResponse(e.args[0])191 if e.response is not None:
192 detail = e.response.content
193 if isinstance(detail, bytes):
194 detail = detail.decode("UTF-8", errors="replace")
195 else:
196 detail = None
197 raise BadUploadResponse(e.args[0], detail=detail)
189 finally:198 finally:
190 lfa.close()199 lfa.close()
191200
192201
=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py 2017-04-03 14:43:22 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py 2017-04-18 21:59:36 +0000
@@ -239,7 +239,8 @@
239 self.assertContentEqual([], snapbuild.store_upload_jobs)239 self.assertContentEqual([], snapbuild.store_upload_jobs)
240 job = SnapStoreUploadJob.create(snapbuild)240 job = SnapStoreUploadJob.create(snapbuild)
241 client = FakeSnapStoreClient()241 client = FakeSnapStoreClient()
242 client.upload.failure = BadUploadResponse("Failed to upload")242 client.upload.failure = BadUploadResponse(
243 "Failed to upload", detail="The proxy exploded.\n")
243 self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))244 self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
244 with dbuser(config.ISnapStoreUploadJobSource.dbuser):245 with dbuser(config.ISnapStoreUploadJobSource.dbuser):
245 JobRunner([job]).runAll()246 JobRunner([job]).runAll()
@@ -277,6 +278,8 @@
277 build_url, footer)278 build_url, footer)
278 self.assertWebhookDeliveries(279 self.assertWebhookDeliveries(
279 snapbuild, ["Pending", "Failed to upload"])280 snapbuild, ["Pending", "Failed to upload"])
281 self.assertIn(
282 ("error_detail", "The proxy exploded.\n"), job.getOopsVars())
280283
281 def test_run_scan_pending_retries(self):284 def test_run_scan_pending_retries(self):
282 # A run that finds that the store has not yet finished scanning the285 # A run that finds that the store has not yet finished scanning the
283286
=== modified file 'lib/lp/snappy/tests/test_snapstoreclient.py'
--- lib/lp/snappy/tests/test_snapstoreclient.py 2017-02-27 12:01:08 +0000
+++ lib/lp/snappy/tests/test_snapstoreclient.py 2017-04-18 21:59:36 +0000
@@ -48,6 +48,7 @@
48 BadRequestPackageUploadResponse,48 BadRequestPackageUploadResponse,
49 BadScanStatusResponse,49 BadScanStatusResponse,
50 BadSearchResponse,50 BadSearchResponse,
51 BadUploadResponse,
51 ISnapStoreClient,52 ISnapStoreClient,
52 ReleaseFailedResponse,53 ReleaseFailedResponse,
53 ScanFailedResponse,54 ScanFailedResponse,
@@ -445,6 +446,25 @@
445 store_secrets["discharge"],446 store_secrets["discharge"],
446 snapbuild.snap.store_secrets["discharge"])447 snapbuild.snap.store_secrets["discharge"])
447448
449 def test_upload_file_error(self):
450 @urlmatch(path=r".*/unscanned-upload/$")
451 def unscanned_upload_handler(url, request):
452 return {
453 "status_code": 502,
454 "reason": "Proxy Error",
455 "content": b"The proxy exploded.\n",
456 }
457
458 store_secrets = self._make_store_secrets()
459 snapbuild = self.makeUploadableSnapBuild(store_secrets=store_secrets)
460 transaction.commit()
461 with dbuser(config.ISnapStoreUploadJobSource.dbuser):
462 with HTTMock(unscanned_upload_handler):
463 err = self.assertRaises(
464 BadUploadResponse, self.client.upload, snapbuild)
465 self.assertEqual("502 Server Error: Proxy Error", str(err))
466 self.assertEqual(b"The proxy exploded.\n", err.detail)
467
448 def test_refresh_discharge_macaroon(self):468 def test_refresh_discharge_macaroon(self):
449 store_secrets = self._make_store_secrets()469 store_secrets = self._make_store_secrets()
450 snap = self.factory.makeSnap(470 snap = self.factory.makeSnap(