Use CodeMessageException subclass instead

Parse json errors from get_json client methods and throw special
errors.

4 files changed, 51 insertions, 61 deletions

diff --git a/synapse/api/errors.py b/synapse/api/errors.py
index 6fbd5d6876..d0dfa959dc 100644
--- a/synapse/api/errors.py
+++ b/synapse/api/errors.py
@@ -66,6 +66,17 @@ class CodeMessageException(RuntimeError):
         return cs_error(self.msg)
 
 
+class MatrixCodeMessageException(CodeMessageException):
+    """An error from a general matrix endpoint, eg. from a proxied Matrix API call.
+
+    Attributes:
+        errcode (str): Matrix error code e.g 'M_FORBIDDEN'
+    """
+    def __init__(self, code, msg, errcode=Codes.UNKNOWN):
+        super(MatrixCodeMessageException, self).__init__(code, msg)
+        self.errcode = errcode
+
+
 class SynapseError(CodeMessageException):
     """A base exception type for matrix errors which have an errcode and error
     message (as well as an HTTP status code).
diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index 41f978d990..8b407a307c 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -18,7 +18,7 @@
 from twisted.internet import defer
 
 from synapse.api.errors import (
-    CodeMessageException
+    MatrixCodeMessageException, CodeMessageException
 )
 from ._base import BaseHandler
 from synapse.util.async import run_on_reactor
@@ -35,7 +35,7 @@ class IdentityHandler(BaseHandler):
     def __init__(self, hs):
         super(IdentityHandler, self).__init__(hs)
 
-        self.proxy_client = hs.get_matrix_proxy_client()
+        self.http_client = hs.get_simple_http_client()
 
         self.trusted_id_servers = set(hs.config.trusted_third_party_id_servers)
         self.trust_any_id_server_just_for_testing_do_not_use = (
@@ -83,13 +83,16 @@ class IdentityHandler(BaseHandler):
 
         data = {}
         try:
-            data = yield self.proxy_client.get_json(
-                "https://%s%s" % (
+            data = yield self.http_client.get_json(
+                "http://%s%s" % (
                     id_server,
                     "/_matrix/identity/api/v1/3pid/getValidated3pid"
                 ),
                 {'sid': creds['sid'], 'client_secret': client_secret}
             )
+        except MatrixCodeMessageException as e:
+            logger.info("getValidated3pid failed with Matrix error: %r", e)
+            raise SynapseError(e.code, e.msg, e.errcode)
         except CodeMessageException as e:
             data = json.loads(e.msg)
 
@@ -118,8 +121,8 @@ class IdentityHandler(BaseHandler):
             raise SynapseError(400, "No client_secret in creds")
 
         try:
-            data = yield self.proxy_client.post_urlencoded_get_json(
-                "https://%s%s" % (
+            data = yield self.http_client.post_urlencoded_get_json(
+                "http://%s%s" % (
                     id_server, "/_matrix/identity/api/v1/3pid/bind"
                 ),
                 {
@@ -151,14 +154,17 @@ class IdentityHandler(BaseHandler):
         params.update(kwargs)
 
         try:
-            data = yield self.proxy_client.post_json_get_json(
-                "https://%s%s" % (
+            data = yield self.http_client.post_json_get_json(
+                "http://%s%s" % (
                     id_server,
                     "/_matrix/identity/api/v1/validate/email/requestToken"
                 ),
                 params
             )
             defer.returnValue(data)
+        except MatrixCodeMessageException as e:
+            logger.info("Proxied requestToken failed with Matrix error: %r", e)
+            raise SynapseError(e.code, e.msg, e.errcode)
         except CodeMessageException as e:
             logger.info("Proxied requestToken failed: %r", e)
             raise e
@@ -185,14 +191,17 @@ class IdentityHandler(BaseHandler):
         params.update(kwargs)
 
         try:
-            data = yield self.proxy_client.post_json_get_json(
-                "https://%s%s" % (
+            data = yield self.http_client.post_json_get_json(
+                "http://%s%s" % (
                     id_server,
                     "/_matrix/identity/api/v1/validate/msisdn/requestToken"
                 ),
                 params
             )
             defer.returnValue(data)
+        except MatrixCodeMessageException as e:
+            logger.info("Proxied requestToken failed with Matrix error: %r", e)
+            raise SynapseError(e.code, e.msg, e.errcode)
         except CodeMessageException as e:
             logger.info("Proxied requestToken failed: %r", e)
             raise e
diff --git a/synapse/http/client.py b/synapse/http/client.py
index df8c3e3c2c..57a49b2827 100644
--- a/synapse/http/client.py
+++ b/synapse/http/client.py
@@ -16,7 +16,7 @@ from OpenSSL import SSL
 from OpenSSL.SSL import VERIFY_NONE
 
 from synapse.api.errors import (
-    CodeMessageException, SynapseError, Codes,
+    CodeMessageException, MatrixCodeMessageException, SynapseError, Codes,
 )
 from synapse.util.logcontext import preserve_context_over_fn
 import synapse.metrics
@@ -145,8 +145,10 @@ class SimpleHttpClient(object):
 
         body = yield preserve_context_over_fn(readBody, response)
 
-        if response.code / 100 >= 4:
-            raise CodeMessageException(response.code, body)
+        if 200 <= response.code < 300:
+            defer.returnValue(json.loads(body))
+        else:
+            raise self._exceptionFromFailedRequest(response, body)
 
         defer.returnValue(json.loads(body))
 
@@ -168,7 +170,11 @@ class SimpleHttpClient(object):
             error message.
         """
         body = yield self.get_raw(uri, args)
-        defer.returnValue(json.loads(body))
+
+        if 200 <= response.code < 300:
+            defer.returnValue(json.loads(body))
+        else:
+            raise self._exceptionFromFailedRequest(response, body)
 
     @defer.inlineCallbacks
     def put_json(self, uri, json_body, args={}):
@@ -249,6 +255,16 @@ class SimpleHttpClient(object):
         else:
             raise CodeMessageException(response.code, body)
 
+    def _exceptionFromFailedRequest(self, response, body):
+        try:
+            jsonBody = json.loads(body)
+            errcode = jsonBody['errcode']
+            error = jsonBody['error']
+            return MatrixCodeMessageException(response.code, error, errcode)
+        except e:
+            print e
+            return CodeMessageException(response.code, body)
+
     # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient.
     # The two should be factored out.
 
@@ -309,46 +325,6 @@ class SimpleHttpClient(object):
         defer.returnValue((length, headers, response.request.absoluteURI, response.code))
 
 
-class MatrixProxyClient(object):
-    """
-    An HTTP client that proxies other Matrix endpoints, ie. if the remote endpoint
-    returns Matrix-style error response, this will raise the appropriate SynapseError
-    """
-    def __init__(self, hs):
-        self.simpleHttpClient = SimpleHttpClient(hs)
-
-    @defer.inlineCallbacks
-    def post_json_get_json(self, uri, post_json):
-        try:
-            result = yield self.simpleHttpClient.post_json_get_json(uri, post_json)
-            defer.returnValue(result)
-        except CodeMessageException as cme:
-            ex = self._tryGetMatrixError(cme)
-            if ex is not None:
-                raise ex
-            raise cme
-
-    @defer.inlineCallbacks
-    def get_json(self, uri, args={}):
-        try:
-            result = yield self.simpleHttpClient.get_json(uri, args)
-            defer.returnValue(result)
-        except CodeMessageException as cme:
-            ex = self._tryGetMatrixError(cme)
-            if ex is not None:
-                raise ex
-            raise cme
-
-    def _tryGetMatrixError(self, codeMessageException):
-        try:
-            errbody = json.loads(codeMessageException.msg)
-            errcode = errbody['errcode']
-            errtext = errbody['error']
-            return SynapseError(codeMessageException.code, errtext, errcode)
-        except:
-            return None
-
-
 # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient.
 # The two should be factored out.
 
diff --git a/synapse/server.py b/synapse/server.py
index 8325d77a7a..12754c89ae 100644
--- a/synapse/server.py
+++ b/synapse/server.py
@@ -49,9 +49,7 @@ from synapse.handlers.events import EventHandler, EventStreamHandler
 from synapse.handlers.initial_sync import InitialSyncHandler
 from synapse.handlers.receipts import ReceiptsHandler
 from synapse.handlers.read_marker import ReadMarkerHandler
-from synapse.http.client import (
-    SimpleHttpClient, InsecureInterceptableContextFactory, MatrixProxyClient
-)
+from synapse.http.client import SimpleHttpClient, InsecureInterceptableContextFactory
 from synapse.http.matrixfederationclient import MatrixFederationHttpClient
 from synapse.notifier import Notifier
 from synapse.push.pusherpool import PusherPool
@@ -130,7 +128,6 @@ class HomeServer(object):
         'filtering',
         'http_client_context_factory',
         'simple_http_client',
-        'matrix_proxy_client',
         'media_repository',
         'federation_transport_client',
         'federation_sender',
@@ -193,9 +190,6 @@ class HomeServer(object):
     def build_simple_http_client(self):
         return SimpleHttpClient(self)
 
-    def build_matrix_proxy_client(self):
-        return MatrixProxyClient(self)
-
     def build_v1auth(self):
         orf = Auth(self)
         # Matrix spec makes no reference to what HTTP status code is returned,