Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Implement MSC3848: Introduce errcodes for specific event sending failures #13343

Merged
merged 19 commits into from
Jul 27, 2022
Merged

Implement MSC3848: Introduce errcodes for specific event sending failures #13343

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
472d91d
Saving changes
Half-Shot Jul 20, 2022
366bc6d
Add unstable spec auth errors
Half-Shot Jul 20, 2022
60c9cf2
lint
Half-Shot Jul 20, 2022
4cdb8a6
prefixing
Half-Shot Jul 20, 2022
f3defc6
notes
Half-Shot Jul 20, 2022
8d6bafa
Drop mistakenly committed changes
Half-Shot Jul 20, 2022
b302d34
Add NOT_JOINED
Half-Shot Jul 22, 2022
8d08d0f
Use msc namespace
Half-Shot Jul 25, 2022
ae053f4
namespace
Half-Shot Jul 25, 2022
eb870dd
Make errors configurable
Half-Shot Jul 25, 2022
1a97541
tidy up if
Half-Shot Jul 25, 2022
1d34f6b
make lint happy
Half-Shot Jul 25, 2022
4f7629b
Pass in config to all error types
Half-Shot Jul 27, 2022
5b3ef13
Appease python
Half-Shot Jul 27, 2022
780ed56
reorder
Half-Shot Jul 27, 2022
7ff61c9
lint again
Half-Shot Jul 27, 2022
584351b
Make homeserver config optional
Half-Shot Jul 27, 2022
1c19e5f
Fix optionals
Half-Shot Jul 27, 2022
c430045
mark as optional to appease code
Half-Shot Jul 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions synapse/api/errors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def __init__(
else:
self._additional_fields = dict(additional_fields)

def error_dict(self, config: "HomeServerConfig") -> "JsonDict":
def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, **self._additional_fields)


Expand Down Expand Up @@ -220,7 +220,7 @@ def __init__(self, msg: str, consent_uri: str):
)
self._consent_uri = consent_uri

def error_dict(self, config: "HomeServerConfig") -> "JsonDict":
def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, consent_uri=self._consent_uri)


Expand Down Expand Up @@ -333,9 +333,9 @@ def __init__(
self.previous_errcode = previous_errcode
super().__init__(code, msg, errcode, additional_fields)

def error_dict(self, config: "HomeServerConfig") -> "JsonDict":
def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
fields = {}
if config.experimental.msc3848_enabled:
if config != None and config.experimental.msc3848_enabled:
fields["org.matrix.msc3848.unstable.errcode"] = self.errcode
return cs_error(
self.msg,
Expand Down Expand Up @@ -376,7 +376,7 @@ def __init__(
super().__init__(msg=msg, errcode="M_UNKNOWN_TOKEN")
self._soft_logout = soft_logout

def error_dict(self, config: "HomeServerConfig") -> "JsonDict":
def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
d = super().error_dict(config)
d["soft_logout"] = self._soft_logout
return d
Expand All @@ -400,7 +400,7 @@ def __init__(
self.limit_type = limit_type
super().__init__(code, msg, errcode=errcode)

def error_dict(self, config: "HomeServerConfig") -> "JsonDict":
def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(
self.msg,
self.errcode,
Expand Down Expand Up @@ -435,7 +435,7 @@ def __init__(
super().__init__(code, msg, errcode)
self.error_url = error_url

def error_dict(self, config: "HomeServerConfig") -> "JsonDict":
def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, error_url=self.error_url)


Expand All @@ -452,7 +452,7 @@ def __init__(
super().__init__(code, msg, errcode)
self.retry_after_ms = retry_after_ms

def error_dict(self, config: "HomeServerConfig") -> "JsonDict":
def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, retry_after_ms=self.retry_after_ms)


Expand All @@ -467,7 +467,7 @@ def __init__(self, current_version: str):
super().__init__(403, "Wrong room_keys version", Codes.WRONG_ROOM_KEYS_VERSION)
self.current_version = current_version

def error_dict(self, config: "HomeServerConfig") -> "JsonDict":
def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, current_version=self.current_version)


Expand Down Expand Up @@ -507,7 +507,7 @@ def __init__(self, room_version: str):

self._room_version = room_version

def error_dict(self, config: "HomeServerConfig") -> "JsonDict":
def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, room_version=self._room_version)


Expand Down Expand Up @@ -553,7 +553,7 @@ def __init__(self, content_keep_ms: Optional[int] = None):
)
self.content_keep_ms = content_keep_ms

def error_dict(self, config: "HomeServerConfig") -> "JsonDict":
def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
extra = {}
if self.content_keep_ms is not None:
extra = {"fi.mau.msc2815.content_keep_ms": self.content_keep_ms}
Expand Down
12 changes: 3 additions & 9 deletions synapse/http/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,11 @@ def is_method_cancellable(method: Callable[..., Any]) -> bool:


def return_json_error(
f: failure.Failure, request: SynapseRequest, config: HomeServerConfig
f: failure.Failure, request: SynapseRequest, config: HomeServerConfig|None
) -> None:
"""Sends a JSON error response to clients."""

if f.check(UnstableSpecAuthError):
# mypy doesn't understand that f.check asserts the type.
exc: UnstableSpecAuthError = f.value # type: ignore
error_code = exc.code
error_dict = exc.error_dict(config)
logger.info("%s SynapseError: %s - %s", request, error_code, exc.msg)
elif f.check(SynapseError):
if f.check(SynapseError):
# mypy doesn't understand that f.check asserts the type.
exc: SynapseError = f.value # type: ignore
error_code = exc.code
Expand Down Expand Up @@ -459,7 +453,7 @@ def _send_error_response(
request: SynapseRequest,
) -> None:
"""Implements _AsyncResource._send_error_response"""
return_json_error(f, request)
return_json_error(f, request, None)
Copy link
Collaborator Author

@Half-Shot Half-Shot Jul 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't figure out a non-invasive way of providing hs to this class so instead here we just don't support unstable error codes on anything directly subclassing DirectServeJsonResource.

A cursory glance at the code seems to indicates this isn't used by any REST API classes that interact with the auth parts of Synapse, so it's unlikely we would raise an unstable error in those parts. Most APIs directly or indirectly subclass JsonResource, which does provide a hs object.



@attr.s(slots=True, frozen=True, auto_attribs=True)
Expand Down
3 changes: 2 additions & 1 deletion tests/rest/client/test_third_party_rules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from synapse.api.constants import EventTypes, LoginType, Membership
from synapse.api.errors import SynapseError
from synapse.api.room_versions import RoomVersion
from synapse.config.homeserver import HomeServerConfig
from synapse.events import EventBase
from synapse.events.third_party_rules import load_legacy_third_party_event_rules
from synapse.rest import admin
Expand Down Expand Up @@ -185,7 +186,7 @@ def test_third_party_rules_workaround_synapse_errors_pass_through(self) -> None:
"""

class NastyHackException(SynapseError):
def error_dict(self, config) -> JsonDict:
def error_dict(self, config: HomeServerConfig) -> JsonDict:
"""
This overrides SynapseError's `error_dict` to nastily inject
JSON into the error response.
Expand Down