Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch failures to decode JSON in /register #463

Merged
merged 3 commits into from
Nov 10, 2021

Conversation

DMRobertson
Copy link
Contributor

Like #456, but for another error case.

If the remote homeserver fails to give us sensible response, that's not
a problem in our application. We can continue to serve everyone else. We
should tell the requester what happened and log a warning, but not an
error to Sentry.

Most of the examples I saw in Sentry were 404 responses serving up a
generic "page not found" body.

Like #456, but for another error case.

If the remote homeserver fails to give us sensible response, that's not
a problem in our application. We can continue to serve everyone else. We
should tell the requester what happened and log a warning, but not an
error to Sentry.

Most of the examples I saw in Sentry were 404 responses serving up a
generic "page not found" body.
@DMRobertson DMRobertson requested a review from a team as a code owner November 9, 2021 17:42
@DMRobertson DMRobertson force-pushed the dmr/catch-json-decode-error branch from 9c03475 to f1ae64f Compare November 9, 2021 17:48
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Seems reasonable.

@DMRobertson DMRobertson merged commit ad02987 into main Nov 10, 2021
@DMRobertson DMRobertson deleted the dmr/catch-json-decode-error branch November 10, 2021 15:46
DMRobertson pushed a commit that referenced this pull request Nov 11, 2021
Addresses
https://sentry.matrix.org/sentry/sydent/issues/235004/?query=is%3Aunresolved%20event.timestamp%3A%3E%3D2021-11-03T10%3A00%3A00

That issue is rare---it's only occurred twice---but since it's so close
to #456 I thought I may as well throw it in too.

(I suppose this might encounter the problem in #463 as well, though we
haven't seen it in Sentry. I think unbinds are just much less used that
registers. I don't know if there's a better way to handle all this
duplication though?)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants