/synapse

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

Already on GitHub? Sign in to your account

Fix rejection of invites to unreachable servers #2145

Merged
merged 5 commits into from Apr 24, 2017

Conversation

Reviewers

@erikjohnston erikjohnston

Assignees

@erikjohnston erikjohnston

Labels
stuck-invite
Projects
None yet
Milestone
No milestone
3 participants
@richvdh @erikjohnston @thekyriarchy
@richvdh
Member

richvdh commented Apr 21, 2017 • edited

When we get a 500 from a remote server, or it is unreachable, mark the invite as locally rejected.

I'm slightly undecided as to whether this is a sensible thing to do; the problem is, of course, that if the remote server was having an off day rather than being permanently out of service, we're going to end up with the room being out of sync. Still, I think that's better than the current situation of not being able to do anything with an invite from an HS which has since disappeared.

This fixes #761

richvdh added some commits Apr 20, 2017

@richvdh
Fix some lies, and other clarifications, in docstrings 
The documentation on get_json has been wrong ever since the very first commit
to synapse...
4903ccf
@richvdh
Remove redundant function 
inline `reject_remote_invite`, which only existed to make tracing the callflow
more difficult.
736b9a4
@richvdh
Broaden the conditions for locally_rejecting invites 
The logic for marking invites as locally rejected was all well and good, but
didn't happen when the remote server returned a 500, or wasn't reachable, or
had no DNS, or whatever.

Just expand the except clause to catch everything.

Fixes #761.
838810b
@richvdh
Remove redundant try/except clauses 
The `except SynapseError` clauses were pointless because the wrapped functions
would never throw a `SynapseError` (they either throw a `CodeMessageException`
or a `RuntimeError`).

The `except CodeMessageException` is now also pointless because the caller
treats all exceptions equally, so we may as well just throw the
`CodeMessageException`.

Some checks were not successful

3 errored and 5 successful checks
@matrixbot
Sytest Dendron (Commit) — Build #1981 origin/rav/reject_invite_to_unreachable_server failed in 9 min 56 sec
Details
@matrixbot
Sytest Postgres (Commit) — Build #2811 origin/rav/reject_invite_to_unreachable_server failed in 7 min 19 sec
Details
@matrixbot
Sytest SQLite (Commit) — Build #2880 origin/rav/reject_invite_to_unreachable_server failed in 6 min 30 sec
Details
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
0cdb32f
@richvdh
Try harder when sending leave events 
When we're rejecting invites, ignore the backoff data, so that we have a better
chance of not getting the room out of sync.

Some checks were not successful

3 errored and 5 successful checks
@matrixbot
Sytest Dendron (Commit) — Build #1982 origin/rav/reject_invite_to_unreachable_server failed in 9 min 14 sec
Details
@matrixbot
Sytest Postgres (Commit) — Build #2812 origin/rav/reject_invite_to_unreachable_server failed in 8 min 17 sec
Details
@matrixbot
Sytest SQLite (Commit) — Build #2881 origin/rav/reject_invite_to_unreachable_server failed in 6 min 38 sec
Details
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
91b3981
@richvdh
Member

richvdh commented Apr 21, 2017

Tests at matrix-org/sytest#353

@richvdh richvdh assigned erikjohnston Apr 21, 2017

@erikjohnston
Owner

erikjohnston commented Apr 24, 2017

I'm surprised that the merged PR tests are fine and the commit ones aren't? Given the commit ones should be using the sytest branch of your PR
@richvdh
Member

richvdh commented Apr 24, 2017

retest this please
@richvdh
Member

richvdh commented Apr 24, 2017

except that doesn't trigger the right builds
@richvdh
Member

richvdh commented Apr 24, 2017

the fails seem reproducible so am investigating further
@richvdh
Member

richvdh commented Apr 24, 2017

right; the problem was in the tests. Now fixed in matrix-org/sytest@a7e9d38.

Merging this since it seems fine.

@richvdh richvdh merged commit 30f7bfa into develop Apr 24, 2017
5 of 8 checks passed

5 of 8 checks passed

Sytest Dendron (Commit) Build #1982 origin/rav/reject_invite_to_unreachable_server failed in 9 min 14 sec
Details
Sytest Postgres (Commit) Build #2812 origin/rav/reject_invite_to_unreachable_server failed in 8 min 17 sec
Details
Sytest SQLite (Commit) Build #2881 origin/rav/reject_invite_to_unreachable_server failed in 6 min 38 sec
Details
Sytest Dendron (Merged PR) Build finished.
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Merged PR) Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

This was referenced Apr 28, 2017

Closed
Closed

@richvdh richvdh added the stuck-invite label May 3, 2017

@Matrixcoffee Matrixcoffee referenced this pull request in vector-im/riot-android May 17, 2017

Closed

Allow invite notification to be removed if cannot reject/accept (server unreachable) #1223

psaavedra added a commit to psaavedra/synapse that referenced this pull request May 19, 2017

@psaavedra
Merge tag 'v0.21.0' into v0.21.0_no_federate_by_default 
Changes in synapse v0.21.0 (2017-05-18)
=======================================

No changes since v0.21.0-rc3

Changes in synapse v0.21.0-rc3 (2017-05-17)
===========================================

Features:

* Add per user rate-limiting overrides (PR #2208)
* Add config option to limit maximum number of events requested by ``/sync``
  and ``/messages`` (PR #2221) Thanks to @psaavedra!

Changes:

* Various small performance fixes (PR #2201, #2202, #2224, #2226, #2227, #2228,
  #2229)
* Update username availability checker API (PR #2209, #2213)
* When purging, don't de-delta state groups we're about to delete (PR #2214)
* Documentation to check synapse version (PR #2215) Thanks to @hamber-dick!
* Add an index to event_search to speed up purge history API (PR #2218)

Bug fixes:

* Fix API to allow clients to upload one-time-keys with new sigs (PR #2206)

Changes in synapse v0.21.0-rc2 (2017-05-08)
===========================================

Changes:

* Always mark remotes as up if we receive a signed request from them (PR #2190)

Bug fixes:

* Fix bug where users got pushed for rooms they had muted (PR #2200)

Changes in synapse v0.21.0-rc1 (2017-05-08)
===========================================

Features:

* Add username availability checker API (PR #2183)
* Add read marker API (PR #2120)

Changes:

* Enable guest access for the 3pl/3pid APIs (PR #1986)
* Add setting to support TURN for guests (PR #2011)
* Various performance improvements (PR #2075, #2076, #2080, #2083, #2108,
  #2158, #2176, #2185)
* Make synctl a bit more user friendly (PR #2078, #2127) Thanks @APwhitehat!
* Replace HTTP replication with TCP replication (PR #2082, #2097, #2098,
  #2099, #2103, #2014, #2016, #2115, #2116, #2117)
* Support authenticated SMTP (PR #2102) Thanks @DanielDent!
* Add a counter metric for successfully-sent transactions (PR #2121)
* Propagate errors sensibly from proxied IS requests (PR #2147)
* Add more granular event send metrics (PR #2178)

Bug fixes:

* Fix nuke-room script to work with current schema (PR #1927) Thanks
  @zuckschwerdt!
* Fix db port script to not assume postgres tables are in the public schema
  (PR #2024) Thanks @jerrykan!
* Fix getting latest device IP for user with no devices (PR #2118)
* Fix rejection of invites to unreachable servers (PR #2145)
* Fix code for reporting old verify keys in synapse (PR #2156)
* Fix invite state to always include all events (PR #2163)
* Fix bug where synapse would always fetch state for any missing event (PR #2170)
* Fix a leak with timed out HTTP connections (PR #2180)
* Fix bug where we didn't time out HTTP requests to ASes  (PR #2192)

Docs:

* Clarify doc for SQLite to PostgreSQL port (PR #1961) Thanks @benhylau!
* Fix typo in synctl help (PR #2107) Thanks @HarHarLinks!
* ``web_client_location`` documentation fix (PR #2131) Thanks @matthewjwolff!
* Update README.rst with FreeBSD changes (PR #2132) Thanks @feld!
* Clarify setting up metrics (PR #2149) Thanks @encks!
badd30b

@uhoreg uhoreg referenced this pull request in vector-im/riot-web Jun 7, 2017

Open

Cannot reject invite from a dead/malfunctioning homeserver #4225

@thekyriarchy

thekyriarchy commented Jul 11, 2017

i still have an invite that i cannot accept or reject because the room no longer exists

@thekyriarchy thekyriarchy referenced this pull request Jul 11, 2017

Closed

can't act on invite to inexistent room #2293

@richvdh richvdh deleted the rav/reject_invite_to_unreachable_server branch Oct 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment