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

Don't fail requests to unbind 3pids for non supporting ID servers #3667

Merged
merged 2 commits into from
Aug 15, 2018

Conversation

erikjohnston
Copy link
Member

Older identity servers may not support the unbind 3pid request, so we
shouldn't fail the requests if we received one of 400/404/501. The
request still fails if we receive e.g. 500 responses, allowing clients
to retry requests on transient identity server errors that otherwise do
support the API.

# The remote server probably doesn't support unbinding (yet)
defer.returnValue(False)
else:
raise SynapseError(502, "Failed to contact identity server")
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we include some info about the error we got from the IS?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, possibly. Not sure what we'd want to copy? @richvdh did you have opinions about how this is meant to work?

Copy link
Member

Choose a reason for hiding this comment

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

err, not really. It feels like it's probably not something a user is going to do much about, so I'm not sure there is much value in including it in the response.

What we probably should do is log the error.

Older identity servers may not support the unbind 3pid request, so we
shouldn't fail the requests if we received one of 400/404/501. The
request still fails if we receive e.g. 500 responses, allowing clients
to retry requests on transient identity server errors that otherwise do
support the API.

Fixes #3661
@erikjohnston erikjohnston assigned richvdh and unassigned erikjohnston Aug 9, 2018
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm

@richvdh richvdh assigned erikjohnston and unassigned richvdh Aug 13, 2018
@erikjohnston erikjohnston merged commit fef2e65 into develop Aug 15, 2018
turt2live added a commit to turt2live/matrix-doc that referenced this pull request Aug 27, 2018
@erikjohnston erikjohnston deleted the erikj/fixup_unbind branch September 20, 2018 13:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants