Improve errors when MAS contacts the Synapse homeserver #2794
Conversation
Deploying matrix-authentication-service-docs with
|
| Latest commit: |
6351613
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6f4d6e84.matrix-authentication-service-docs.pages.dev |
| Branch Preview URL: | https://rei-homeserver-error-improve.matrix-authentication-service-docs.pages.dev |
reivilibre
force-pushed
the
rei/homeserver_error_improvements
branch
from
fe137dd to
0f94a93
Compare
crates/matrix-synapse/src/error.rs
Outdated
| fn description(&self) -> &str { | ||
| if let Some(matrix_error) = &self.matrix_error { | ||
| &matrix_error.error | ||
| } else { | ||
| "Homeserver Error" | ||
| } | ||
| } |
There was a problem hiding this comment.
description has been deprecated for a while, it's delegated to Debug/Display now
There was a problem hiding this comment.
I think you still have to implement it? Maybe I should have just hardcoded a "don't description() me, you silly!" on it but ehh
There was a problem hiding this comment.
looks like the stdlib already does that for you :)
There was a problem hiding this comment.
huh ok, weird that it added it (rust analyser?) for implementation then. Maybe it just always does that and I'm used to IntelliJ-Rust too much
crates/matrix-synapse/src/lib.rs
Outdated
| #[derive(Deserialize)] | ||
| struct UsernameAvailableResponse { | ||
| available: bool, | ||
| } |
There was a problem hiding this comment.
The other response structures are defined at the top of the file, for consitency could you move that up there?
There was a problem hiding this comment.
sure. It's just a personal style I like for these single-use structs, to make them function-local, but yeah indeed there are others here so let's do that
crates/matrix-synapse/src/error.rs
Outdated
| fn description(&self) -> &str { | ||
| if let Some(matrix_error) = &self.matrix_error { | ||
| &matrix_error.error | ||
| } else { | ||
| "Homeserver Error" | ||
| } | ||
| } |
There was a problem hiding this comment.
looks like the stdlib already does that for you :)
Using `#[source]` is unnatural here because it makes it look like two distinct errors (one being a cause of the other), when in reality it is just one error, with 2 parts. Using `Display` formatting for that leads to a more natural error.
Not strictly required, but does two things: - documents what kind of function is expected - provides a small extra amount of type enforcement at the call site, rather than later on when you find the result doesn't implement Service
Nothing major, just a quality of life improvement so you don't have to repetitively write out what a HTTP error is
…etails' section The extra whitespace was probably unintentional and makes the error harder to read, particularly when it wraps onto a new line unnecessarily
…-synapse support module
reivilibre
force-pushed
the
rei/homeserver_error_improvements
branch
from
3dcda7f to
6351613
Compare
An example of a failed registration now looks like:
Before the only detail this gave you was the headline message repeated a few times.
The logs now say
Should be commit-by-commit review friendly