Use 502 status code for hash mismatches in diff server (#497)

When using the diff server, you can provide a hash to verify that the two pieces of content you are diffing were correctly received. When that hash doesn’t match, we used to respond with a 500 error, but we now use a 502 error to clarify that the problem is related to the upstream server, not us.

Since that can be a little ambiguous (502 = hash mismatch, but also 502 = upstream server error), this also adds a mechanism for sending extra metadata with the error. Specifically, you can now check `error.type` in the response for a more detailed error code, like `HASH_MISMATCH` or `UPSTREAM_ERROR`.

Fixes #491.

web_monitoring/diffing_server.py

@@ -241,15 +241,24 @@ async def fetch_diffable_content(self, url, expected_hash, query_params):
                     response = error.response
                 else:
                     self.send_error(502,
-                                    reason=f'Received a {error.response.code} status while fetching "{url}": {error}')
+                                    reason=f'Received a {error.response.code} '
+                                           f'status while fetching "{url}": '
+                                           f'{error}',
+                                    extra={'type': 'UPSTREAM_ERROR',
+                                           'url': url,
+                                           'upstream_code': error.response.code})
 
         if response and expected_hash:
             actual_hash = hashlib.sha256(response.body).hexdigest()
             if actual_hash != expected_hash:
                 response = None
-                self.send_error(500,
+                self.send_error(502,
                                 reason=(f'Fetched content at "{url}" does not '
-                                        f'match hash "{expected_hash}".'))
+                                        f'match hash "{expected_hash}".'),
+                                extra={'type': 'HASH_MISMATCH',
+                                       'url': url,
+                                       'expected_hash': expected_hash,
+                                       'actual_hash': actual_hash})
 
         return response
 
@@ -285,6 +294,9 @@ def get_diff_executor(self, reset=False):
 
     def write_error(self, status_code, **kwargs):
         response = {'code': status_code, 'error': self._reason}
+        if 'extra' in kwargs:
+            for key, value in kwargs['extra'].items():
+                response[key] = value
 
         # Handle errors that are allowed to be public
         # TODO: this error filtering should probably be in `send_error()`

web_monitoring/tests/test_diffing_server_exc_handling.py

@@ -300,7 +300,7 @@ def test_validates_bad_hashes(self):
                               'a_hash=f3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855&'
                               f'b=file://{fixture_path("empty.txt")}&'
                               'b_hash=e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855')
-        assert response.code == 500
+        assert response.code == 502
         assert 'hash' in json.loads(response.body)['error']