Be more careful which errors we send back over the C-S API
We really shouldn't be sending all CodeMessageExceptions back over the C-S API; it will include things like 401s which we shouldn't proxy. That means that we need to explicitly turn a few HttpResponseExceptions into SynapseErrors in the federation layer. The effect of the latter is that the matrix errcode will get passed through correctly to calling clients, which might help with some of the random M_UNKNOWN errors when trying to join rooms.
This commit is contained in:
parent
c82ccd3027
commit
fa7dc889f1
|
@ -69,9 +69,6 @@ class CodeMessageException(RuntimeError):
|
||||||
self.code = code
|
self.code = code
|
||||||
self.msg = msg
|
self.msg = msg
|
||||||
|
|
||||||
def error_dict(self):
|
|
||||||
return cs_error(self.msg)
|
|
||||||
|
|
||||||
|
|
||||||
class MatrixCodeMessageException(CodeMessageException):
|
class MatrixCodeMessageException(CodeMessageException):
|
||||||
"""An error from a general matrix endpoint, eg. from a proxied Matrix API call.
|
"""An error from a general matrix endpoint, eg. from a proxied Matrix API call.
|
||||||
|
@ -308,14 +305,6 @@ class LimitExceededError(SynapseError):
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def cs_exception(exception):
|
|
||||||
if isinstance(exception, CodeMessageException):
|
|
||||||
return exception.error_dict()
|
|
||||||
else:
|
|
||||||
logger.error("Unknown exception type: %s", type(exception))
|
|
||||||
return {}
|
|
||||||
|
|
||||||
|
|
||||||
def cs_error(msg, code=Codes.UNKNOWN, **kwargs):
|
def cs_error(msg, code=Codes.UNKNOWN, **kwargs):
|
||||||
""" Utility method for constructing an error response for client-server
|
""" Utility method for constructing an error response for client-server
|
||||||
interactions.
|
interactions.
|
||||||
|
|
|
@ -488,7 +488,7 @@ class FederationClient(FederationBase):
|
||||||
The [Deferred] result of callback, if it succeeds
|
The [Deferred] result of callback, if it succeeds
|
||||||
|
|
||||||
Raises:
|
Raises:
|
||||||
CodeMessageException if the chosen remote server returns a 300/400 code.
|
SynapseError if the chosen remote server returns a 300/400 code.
|
||||||
|
|
||||||
RuntimeError if no servers were reachable.
|
RuntimeError if no servers were reachable.
|
||||||
"""
|
"""
|
||||||
|
@ -504,9 +504,9 @@ class FederationClient(FederationBase):
|
||||||
"Failed to %s via %s: %s",
|
"Failed to %s via %s: %s",
|
||||||
description, destination, e,
|
description, destination, e,
|
||||||
)
|
)
|
||||||
except CodeMessageException as e:
|
except HttpResponseException as e:
|
||||||
if not 500 <= e.code < 600:
|
if not 500 <= e.code < 600:
|
||||||
raise
|
raise SynapseError.from_http_response_exception(e)
|
||||||
else:
|
else:
|
||||||
logger.warn(
|
logger.warn(
|
||||||
"Failed to %s via %s: %i %s",
|
"Failed to %s via %s: %i %s",
|
||||||
|
@ -543,7 +543,7 @@ class FederationClient(FederationBase):
|
||||||
Deferred: resolves to a tuple of (origin (str), event (object))
|
Deferred: resolves to a tuple of (origin (str), event (object))
|
||||||
where origin is the remote homeserver which generated the event.
|
where origin is the remote homeserver which generated the event.
|
||||||
|
|
||||||
Fails with a ``CodeMessageException`` if the chosen remote server
|
Fails with a ``SynapseError`` if the chosen remote server
|
||||||
returns a 300/400 code.
|
returns a 300/400 code.
|
||||||
|
|
||||||
Fails with a ``RuntimeError`` if no servers were reachable.
|
Fails with a ``RuntimeError`` if no servers were reachable.
|
||||||
|
@ -599,7 +599,7 @@ class FederationClient(FederationBase):
|
||||||
giving the serer the event was sent to, ``state`` (?) and
|
giving the serer the event was sent to, ``state`` (?) and
|
||||||
``auth_chain``.
|
``auth_chain``.
|
||||||
|
|
||||||
Fails with a ``CodeMessageException`` if the chosen remote server
|
Fails with a ``SynapseError`` if the chosen remote server
|
||||||
returns a 300/400 code.
|
returns a 300/400 code.
|
||||||
|
|
||||||
Fails with a ``RuntimeError`` if no servers were reachable.
|
Fails with a ``RuntimeError`` if no servers were reachable.
|
||||||
|
@ -673,12 +673,17 @@ class FederationClient(FederationBase):
|
||||||
@defer.inlineCallbacks
|
@defer.inlineCallbacks
|
||||||
def send_invite(self, destination, room_id, event_id, pdu):
|
def send_invite(self, destination, room_id, event_id, pdu):
|
||||||
time_now = self._clock.time_msec()
|
time_now = self._clock.time_msec()
|
||||||
|
try:
|
||||||
code, content = yield self.transport_layer.send_invite(
|
code, content = yield self.transport_layer.send_invite(
|
||||||
destination=destination,
|
destination=destination,
|
||||||
room_id=room_id,
|
room_id=room_id,
|
||||||
event_id=event_id,
|
event_id=event_id,
|
||||||
content=pdu.get_pdu_json(time_now),
|
content=pdu.get_pdu_json(time_now),
|
||||||
)
|
)
|
||||||
|
except HttpResponseException as e:
|
||||||
|
if e.code == 403:
|
||||||
|
raise SynapseError.from_http_response_exception(e)
|
||||||
|
raise
|
||||||
|
|
||||||
pdu_dict = content["event"]
|
pdu_dict = content["event"]
|
||||||
|
|
||||||
|
@ -709,7 +714,7 @@ class FederationClient(FederationBase):
|
||||||
Return:
|
Return:
|
||||||
Deferred: resolves to None.
|
Deferred: resolves to None.
|
||||||
|
|
||||||
Fails with a ``CodeMessageException`` if the chosen remote server
|
Fails with a ``SynapseError`` if the chosen remote server
|
||||||
returns a 300/400 code.
|
returns a 300/400 code.
|
||||||
|
|
||||||
Fails with a ``RuntimeError`` if no servers were reachable.
|
Fails with a ``RuntimeError`` if no servers were reachable.
|
||||||
|
|
|
@ -36,7 +36,6 @@ from synapse.api.errors import (
|
||||||
Codes,
|
Codes,
|
||||||
SynapseError,
|
SynapseError,
|
||||||
UnrecognizedRequestError,
|
UnrecognizedRequestError,
|
||||||
cs_exception,
|
|
||||||
)
|
)
|
||||||
from synapse.http.request_metrics import requests_counter
|
from synapse.http.request_metrics import requests_counter
|
||||||
from synapse.util.caches import intern_dict
|
from synapse.util.caches import intern_dict
|
||||||
|
@ -77,16 +76,13 @@ def wrap_json_request_handler(h):
|
||||||
def wrapped_request_handler(self, request):
|
def wrapped_request_handler(self, request):
|
||||||
try:
|
try:
|
||||||
yield h(self, request)
|
yield h(self, request)
|
||||||
except CodeMessageException as e:
|
except SynapseError as e:
|
||||||
code = e.code
|
code = e.code
|
||||||
if isinstance(e, SynapseError):
|
|
||||||
logger.info(
|
logger.info(
|
||||||
"%s SynapseError: %s - %s", request, code, e.msg
|
"%s SynapseError: %s - %s", request, code, e.msg
|
||||||
)
|
)
|
||||||
else:
|
|
||||||
logger.exception(e)
|
|
||||||
respond_with_json(
|
respond_with_json(
|
||||||
request, code, cs_exception(e), send_cors=True,
|
request, code, e.error_dict(), send_cors=True,
|
||||||
pretty_print=_request_user_agent_is_curl(request),
|
pretty_print=_request_user_agent_is_curl(request),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue