Kill off MatrixCodeMessageException

This code brings the SimpleHttpClient into line with the
MatrixFederationHttpClient by having it raise HttpResponseExceptions when a
request fails (rather than trying to parse for matrix errors and maybe raising
MatrixCodeMessageException).

Then, whenever we were checking for MatrixCodeMessageException and turning them
into SynapseErrors, we now need to check for HttpResponseExceptions and call
to_synapse_error.
This commit is contained in:
Richard van der Hoff 2018-08-01 15:04:50 +01:00
parent 018d75a148
commit 01e93f48ed
5 changed files with 47 additions and 78 deletions

View File

@ -70,17 +70,6 @@ class CodeMessageException(RuntimeError):
self.msg = msg self.msg = 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): class SynapseError(CodeMessageException):
"""A base exception type for matrix errors which have an errcode and error """A base exception type for matrix errors which have an errcode and error
message (as well as an HTTP status code). message (as well as an HTTP status code).

View File

@ -26,7 +26,7 @@ from twisted.internet import defer
from synapse.api.errors import ( from synapse.api.errors import (
CodeMessageException, CodeMessageException,
Codes, Codes,
MatrixCodeMessageException, HttpResponseException,
SynapseError, SynapseError,
) )
@ -85,7 +85,6 @@ class IdentityHandler(BaseHandler):
) )
defer.returnValue(None) defer.returnValue(None)
data = {}
try: try:
data = yield self.http_client.get_json( data = yield self.http_client.get_json(
"https://%s%s" % ( "https://%s%s" % (
@ -94,11 +93,9 @@ class IdentityHandler(BaseHandler):
), ),
{'sid': creds['sid'], 'client_secret': client_secret} {'sid': creds['sid'], 'client_secret': client_secret}
) )
except MatrixCodeMessageException as e: except HttpResponseException as e:
logger.info("getValidated3pid failed with Matrix error: %r", e) logger.info("getValidated3pid failed with Matrix error: %r", e)
raise SynapseError(e.code, e.msg, e.errcode) raise e.to_synapse_error()
except CodeMessageException as e:
data = json.loads(e.msg)
if 'medium' in data: if 'medium' in data:
defer.returnValue(data) defer.returnValue(data)
@ -136,7 +133,7 @@ class IdentityHandler(BaseHandler):
) )
logger.debug("bound threepid %r to %s", creds, mxid) logger.debug("bound threepid %r to %s", creds, mxid)
except CodeMessageException as e: except CodeMessageException as e:
data = json.loads(e.msg) data = json.loads(e.msg) # XXX WAT?
defer.returnValue(data) defer.returnValue(data)
@defer.inlineCallbacks @defer.inlineCallbacks
@ -209,12 +206,9 @@ class IdentityHandler(BaseHandler):
params params
) )
defer.returnValue(data) defer.returnValue(data)
except MatrixCodeMessageException as e: except HttpResponseException 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) logger.info("Proxied requestToken failed: %r", e)
raise e raise e.to_synapse_error()
@defer.inlineCallbacks @defer.inlineCallbacks
def requestMsisdnToken( def requestMsisdnToken(
@ -244,9 +238,6 @@ class IdentityHandler(BaseHandler):
params params
) )
defer.returnValue(data) defer.returnValue(data)
except MatrixCodeMessageException as e: except HttpResponseException 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) logger.info("Proxied requestToken failed: %r", e)
raise e raise e.to_synapse_error()

View File

@ -39,12 +39,7 @@ from twisted.web.client import (
from twisted.web.http import PotentialDataLoss from twisted.web.http import PotentialDataLoss
from twisted.web.http_headers import Headers from twisted.web.http_headers import Headers
from synapse.api.errors import ( from synapse.api.errors import Codes, HttpResponseException, SynapseError
CodeMessageException,
Codes,
MatrixCodeMessageException,
SynapseError,
)
from synapse.http import cancelled_to_request_timed_out_error, redact_uri from synapse.http import cancelled_to_request_timed_out_error, redact_uri
from synapse.http.endpoint import SpiderEndpoint from synapse.http.endpoint import SpiderEndpoint
from synapse.util.async import add_timeout_to_deferred from synapse.util.async import add_timeout_to_deferred
@ -132,6 +127,11 @@ class SimpleHttpClient(object):
Returns: Returns:
Deferred[object]: parsed json Deferred[object]: parsed json
Raises:
HttpResponseException: On a non-2xx HTTP response.
ValueError: if the response was not JSON
""" """
# TODO: Do we ever want to log message contents? # TODO: Do we ever want to log message contents?
@ -155,7 +155,10 @@ class SimpleHttpClient(object):
body = yield make_deferred_yieldable(readBody(response)) body = yield make_deferred_yieldable(readBody(response))
defer.returnValue(json.loads(body)) if 200 <= response.code < 300:
defer.returnValue(json.loads(body))
else:
raise HttpResponseException(response.code, response.phrase, body)
@defer.inlineCallbacks @defer.inlineCallbacks
def post_json_get_json(self, uri, post_json, headers=None): def post_json_get_json(self, uri, post_json, headers=None):
@ -169,6 +172,11 @@ class SimpleHttpClient(object):
Returns: Returns:
Deferred[object]: parsed json Deferred[object]: parsed json
Raises:
HttpResponseException: On a non-2xx HTTP response.
ValueError: if the response was not JSON
""" """
json_str = encode_canonical_json(post_json) json_str = encode_canonical_json(post_json)
@ -193,9 +201,7 @@ class SimpleHttpClient(object):
if 200 <= response.code < 300: if 200 <= response.code < 300:
defer.returnValue(json.loads(body)) defer.returnValue(json.loads(body))
else: else:
raise self._exceptionFromFailedRequest(response, body) raise HttpResponseException(response.code, response.phrase, body)
defer.returnValue(json.loads(body))
@defer.inlineCallbacks @defer.inlineCallbacks
def get_json(self, uri, args={}, headers=None): def get_json(self, uri, args={}, headers=None):
@ -213,14 +219,12 @@ class SimpleHttpClient(object):
Deferred: Succeeds when we get *any* 2xx HTTP response, with the Deferred: Succeeds when we get *any* 2xx HTTP response, with the
HTTP body as JSON. HTTP body as JSON.
Raises: Raises:
On a non-2xx HTTP response. The response body will be used as the HttpResponseException On a non-2xx HTTP response.
error message.
ValueError: if the response was not JSON
""" """
try: body = yield self.get_raw(uri, args, headers=headers)
body = yield self.get_raw(uri, args, headers=headers) defer.returnValue(json.loads(body))
defer.returnValue(json.loads(body))
except CodeMessageException as e:
raise self._exceptionFromFailedRequest(e.code, e.msg)
@defer.inlineCallbacks @defer.inlineCallbacks
def put_json(self, uri, json_body, args={}, headers=None): def put_json(self, uri, json_body, args={}, headers=None):
@ -239,7 +243,9 @@ class SimpleHttpClient(object):
Deferred: Succeeds when we get *any* 2xx HTTP response, with the Deferred: Succeeds when we get *any* 2xx HTTP response, with the
HTTP body as JSON. HTTP body as JSON.
Raises: Raises:
On a non-2xx HTTP response. HttpResponseException On a non-2xx HTTP response.
ValueError: if the response was not JSON
""" """
if len(args): if len(args):
query_bytes = urllib.urlencode(args, True) query_bytes = urllib.urlencode(args, True)
@ -266,10 +272,7 @@ class SimpleHttpClient(object):
if 200 <= response.code < 300: if 200 <= response.code < 300:
defer.returnValue(json.loads(body)) defer.returnValue(json.loads(body))
else: else:
# NB: This is explicitly not json.loads(body)'d because the contract raise HttpResponseException(response.code, response.phrase, body)
# of CodeMessageException is a *string* message. Callers can always
# load it into JSON if they want.
raise CodeMessageException(response.code, body)
@defer.inlineCallbacks @defer.inlineCallbacks
def get_raw(self, uri, args={}, headers=None): def get_raw(self, uri, args={}, headers=None):
@ -287,8 +290,7 @@ class SimpleHttpClient(object):
Deferred: Succeeds when we get *any* 2xx HTTP response, with the Deferred: Succeeds when we get *any* 2xx HTTP response, with the
HTTP body at text. HTTP body at text.
Raises: Raises:
On a non-2xx HTTP response. The response body will be used as the HttpResponseException on a non-2xx HTTP response.
error message.
""" """
if len(args): if len(args):
query_bytes = urllib.urlencode(args, True) query_bytes = urllib.urlencode(args, True)
@ -311,16 +313,7 @@ class SimpleHttpClient(object):
if 200 <= response.code < 300: if 200 <= response.code < 300:
defer.returnValue(body) defer.returnValue(body)
else: else:
raise CodeMessageException(response.code, body) raise HttpResponseException(response.code, response.phrase, body)
def _exceptionFromFailedRequest(self, response, body):
try:
jsonBody = json.loads(body)
errcode = jsonBody['errcode']
error = jsonBody['error']
return MatrixCodeMessageException(response.code, error, errcode)
except (ValueError, KeyError):
return CodeMessageException(response.code, body)
# XXX: FIXME: This is horribly copy-pasted from matrixfederationclient. # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient.
# The two should be factored out. # The two should be factored out.

View File

@ -18,7 +18,7 @@ import re
from twisted.internet import defer from twisted.internet import defer
from synapse.api.errors import MatrixCodeMessageException, SynapseError from synapse.api.errors import HttpResponseException
from synapse.http.servlet import RestServlet, parse_json_object_from_request from synapse.http.servlet import RestServlet, parse_json_object_from_request
from synapse.types import Requester, UserID from synapse.types import Requester, UserID
from synapse.util.distributor import user_joined_room, user_left_room from synapse.util.distributor import user_joined_room, user_left_room
@ -56,11 +56,11 @@ def remote_join(client, host, port, requester, remote_room_hosts,
try: try:
result = yield client.post_json_get_json(uri, payload) result = yield client.post_json_get_json(uri, payload)
except MatrixCodeMessageException as e: except HttpResponseException as e:
# We convert to SynapseError as we know that it was a SynapseError # We convert to SynapseError as we know that it was a SynapseError
# on the master process that we should send to the client. (And # on the master process that we should send to the client. (And
# importantly, not stack traces everywhere) # importantly, not stack traces everywhere)
raise SynapseError(e.code, e.msg, e.errcode) raise e.to_synapse_error()
defer.returnValue(result) defer.returnValue(result)
@ -92,11 +92,11 @@ def remote_reject_invite(client, host, port, requester, remote_room_hosts,
try: try:
result = yield client.post_json_get_json(uri, payload) result = yield client.post_json_get_json(uri, payload)
except MatrixCodeMessageException as e: except HttpResponseException as e:
# We convert to SynapseError as we know that it was a SynapseError # We convert to SynapseError as we know that it was a SynapseError
# on the master process that we should send to the client. (And # on the master process that we should send to the client. (And
# importantly, not stack traces everywhere) # importantly, not stack traces everywhere)
raise SynapseError(e.code, e.msg, e.errcode) raise e.to_synapse_error()
defer.returnValue(result) defer.returnValue(result)
@ -131,11 +131,11 @@ def get_or_register_3pid_guest(client, host, port, requester,
try: try:
result = yield client.post_json_get_json(uri, payload) result = yield client.post_json_get_json(uri, payload)
except MatrixCodeMessageException as e: except HttpResponseException as e:
# We convert to SynapseError as we know that it was a SynapseError # We convert to SynapseError as we know that it was a SynapseError
# on the master process that we should send to the client. (And # on the master process that we should send to the client. (And
# importantly, not stack traces everywhere) # importantly, not stack traces everywhere)
raise SynapseError(e.code, e.msg, e.errcode) raise e.to_synapse_error()
defer.returnValue(result) defer.returnValue(result)
@ -165,11 +165,11 @@ def notify_user_membership_change(client, host, port, user_id, room_id, change):
try: try:
result = yield client.post_json_get_json(uri, payload) result = yield client.post_json_get_json(uri, payload)
except MatrixCodeMessageException as e: except HttpResponseException as e:
# We convert to SynapseError as we know that it was a SynapseError # We convert to SynapseError as we know that it was a SynapseError
# on the master process that we should send to the client. (And # on the master process that we should send to the client. (And
# importantly, not stack traces everywhere) # importantly, not stack traces everywhere)
raise SynapseError(e.code, e.msg, e.errcode) raise e.to_synapse_error()
defer.returnValue(result) defer.returnValue(result)

View File

@ -18,11 +18,7 @@ import re
from twisted.internet import defer from twisted.internet import defer
from synapse.api.errors import ( from synapse.api.errors import CodeMessageException, HttpResponseException
CodeMessageException,
MatrixCodeMessageException,
SynapseError,
)
from synapse.events import FrozenEvent from synapse.events import FrozenEvent
from synapse.events.snapshot import EventContext from synapse.events.snapshot import EventContext
from synapse.http.servlet import RestServlet, parse_json_object_from_request from synapse.http.servlet import RestServlet, parse_json_object_from_request
@ -83,11 +79,11 @@ def send_event_to_master(clock, store, client, host, port, requester, event, con
# If we timed out we probably don't need to worry about backing # If we timed out we probably don't need to worry about backing
# off too much, but lets just wait a little anyway. # off too much, but lets just wait a little anyway.
yield clock.sleep(1) yield clock.sleep(1)
except MatrixCodeMessageException as e: except HttpResponseException as e:
# We convert to SynapseError as we know that it was a SynapseError # We convert to SynapseError as we know that it was a SynapseError
# on the master process that we should send to the client. (And # on the master process that we should send to the client. (And
# importantly, not stack traces everywhere) # importantly, not stack traces everywhere)
raise SynapseError(e.code, e.msg, e.errcode) raise e.to_synapse_error()
defer.returnValue(result) defer.returnValue(result)