Merge pull request #168 from PrivateStorageio/102.parsing-redeem-response-failed

Report "Parsing redeem response failed" better

src/_zkapauthorizer/controller.py

@@ -33,6 +33,7 @@ from functools import (
 )
 from json import (
     dumps,
+    loads,
 )
 from datetime import (
     timedelta,
@@ -75,7 +76,7 @@ from twisted.web.client import (
     Agent,
 )
 from treq import (
-    json_content,
+    content,
 )
 from treq.client import (
     HTTPClient,
@@ -103,6 +104,15 @@ from .model import (
 
 RETRY_INTERVAL = timedelta(milliseconds=1)
 
+@attr.s
+class UnexpectedResponse(Exception):
+    """
+    The issuer responded in an unexpected and unhandled way.
+    """
+    code = attr.ib()
+    body = attr.ib()
+
+
 class AlreadySpent(Exception):
     """
     An attempt was made to redeem a voucher which has already been redeemed.
@@ -524,11 +534,12 @@ class RistrettoRedeemer(object):
             }),
             headers={b"content-type": b"application/json"},
         )
+        response_body = yield content(response)
+
         try:
-            result = yield json_content(response)
+            result = loads(response_body)
         except ValueError:
-            self._log.failure("Parsing redeem response failed", response=response)
-            raise
+            raise UnexpectedResponse(response.code, response_body)
 
         success = result.get(u"success", False)
         if not success:
@@ -933,8 +944,8 @@ class PaymentController(object):
             self._unpaid[voucher] = self.store.now()
         else:
             self._log.error(
-                "Redeeming random tokens for a voucher ({voucher}) failed: {reason}",
-                reason=reason,
+                "Redeeming random tokens for a voucher ({voucher}) failed: {reason!r}",
+                reason=reason.value,
                 voucher=voucher,
             )
             self._error[voucher] = model_Error(

src/_zkapauthorizer/tests/test_controller.py

@@ -86,6 +86,7 @@ from twisted.web.http_headers import (
 from twisted.web.http import (
     UNSUPPORTED_MEDIA_TYPE,
     BAD_REQUEST,
+    INTERNAL_SERVER_ERROR,
 )
 from treq.testing import (
     StubTreq,
@@ -111,6 +112,7 @@ from ..controller import (
     IndexedRedeemer,
     RecordingRedeemer,
     PaymentController,
+    UnexpectedResponse,
     AlreadySpent,
     Unpaid,
     token_count_for_group,
@@ -621,6 +623,39 @@ class RistrettoRedeemerTests(TestCase):
             ),
         )
 
+    @given(voucher_objects(), voucher_counters(), integers(min_value=0, max_value=100))
+    def test_non_json_response(self, voucher, counter, num_tokens):
+        """
+        If the issuer responds with something that isn't JSON then the response is
+        logged and the ``Deferred`` fires with a ``Failure`` wrapping
+        ``UnexpectedResponse``.
+        """
+        issuer = UnexpectedResponseRedemption()
+        treq = treq_for_loopback_ristretto(issuer)
+        redeemer = RistrettoRedeemer(treq, NOWHERE)
+        random_tokens = redeemer.random_tokens_for_voucher(voucher, counter, num_tokens)
+
+        d = redeemer.redeemWithCounter(
+            voucher,
+            counter,
+            random_tokens,
+        )
+
+        self.assertThat(
+            d,
+            failed(
+                AfterPreprocessing(
+                    lambda f: f.value,
+                    Equals(
+                        UnexpectedResponse(
+                            INTERNAL_SERVER_ERROR,
+                            b"Sorry, this server does not behave well.",
+                        ),
+                    ),
+                ),
+            ),
+        )
+
     @given(voucher_objects(), voucher_counters(), integers(min_value=0, max_value=100))
     def test_redemption_denied_alreadyspent(self, voucher, counter, extra_tokens):
         """
@@ -827,6 +862,16 @@ def stub_agent():
     return _StubAgent()
 
 
+class UnexpectedResponseRedemption(Resource):
+    """
+    An ``UnexpectedResponseRedemption`` simulates the Ristretto redemption
+    server but always returns a non-JSON error response.
+    """
+    def render_POST(self, request):
+        request.setResponseCode(INTERNAL_SERVER_ERROR)
+        return b"Sorry, this server does not behave well."
+
+
 class AlreadySpentRedemption(Resource):
     """
     An ``AlreadySpentRedemption`` simulates the Ristretto redemption server