From ecd7e36047d090cdb027f500b0f95a375ba61811 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 13 Feb 2017 13:16:48 +0000 Subject: http txns: Do not cache error responses Previously we did. This meant that, amongst other errors, rate-limiting errors would be cached and prevent messages with that txn ID being sent. --- synapse/rest/client/transactions.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'synapse/rest/client') diff --git a/synapse/rest/client/transactions.py b/synapse/rest/client/transactions.py index efa77b8c51..6396a08035 100644 --- a/synapse/rest/client/transactions.py +++ b/synapse/rest/client/transactions.py @@ -81,7 +81,16 @@ class HttpTransactionCache(object): Deferred which resolves to a tuple of (response_code, response_dict). """ try: - return self.transactions[txn_key][0].observe() + observable = self.transactions[txn_key][0] + if not observable.has_called() or observable.has_succeeded(): + return observable.observe() + # if the request has already been called with a non-2xx status + # (a Twisted failure), remove it from the transaction map. + # This is done to ensure that we don't cache rate-limiting errors, etc. + res = observable.get_result() + if res.value.code >= 300: + del self.transactions[txn_key] + # fall through except (KeyError, IndexError): pass # execute the function instead. -- cgit 1.4.1 From feb15dc99f02e6cb0a84a53e397529c51743f114 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 13 Feb 2017 13:33:12 +0000 Subject: Don't cache errors at all --- synapse/rest/client/transactions.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) (limited to 'synapse/rest/client') diff --git a/synapse/rest/client/transactions.py b/synapse/rest/client/transactions.py index 6396a08035..95376a2fb6 100644 --- a/synapse/rest/client/transactions.py +++ b/synapse/rest/client/transactions.py @@ -81,16 +81,7 @@ class HttpTransactionCache(object): Deferred which resolves to a tuple of (response_code, response_dict). """ try: - observable = self.transactions[txn_key][0] - if not observable.has_called() or observable.has_succeeded(): - return observable.observe() - # if the request has already been called with a non-2xx status - # (a Twisted failure), remove it from the transaction map. - # This is done to ensure that we don't cache rate-limiting errors, etc. - res = observable.get_result() - if res.value.code >= 300: - del self.transactions[txn_key] - # fall through + return self.transactions[txn_key][0].observe() except (KeyError, IndexError): pass # execute the function instead. @@ -101,6 +92,14 @@ class HttpTransactionCache(object): # to the observers. observable = ObservableDeferred(deferred, consumeErrors=True) self.transactions[txn_key] = (observable, self.clock.time_msec()) + + # if the request fails with a Twisted failure, remove it + # from the transaction map. This is done to ensure that we don't + # cache transient errors like rate-limiting errors, etc. + def remove_from_map(err): + del self.transactions[txn_key] + return err + observable.addErrback(remove_from_map) return observable.observe() def _cleanup(self): -- cgit 1.4.1 From 808ddf0ae72ef45d887e00c07ba834d0873ceb8d Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 13 Feb 2017 13:36:15 +0000 Subject: Pop the txn from the map in case it has already been deleted somehow --- synapse/rest/client/transactions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/rest/client') diff --git a/synapse/rest/client/transactions.py b/synapse/rest/client/transactions.py index 95376a2fb6..7b742ca7ae 100644 --- a/synapse/rest/client/transactions.py +++ b/synapse/rest/client/transactions.py @@ -97,7 +97,7 @@ class HttpTransactionCache(object): # from the transaction map. This is done to ensure that we don't # cache transient errors like rate-limiting errors, etc. def remove_from_map(err): - del self.transactions[txn_key] + self.transactions.pop(txn_key, None) return err observable.addErrback(remove_from_map) return observable.observe() -- cgit 1.4.1 From d0497425f81ed21b58046f5f8725fc70ffcc0544 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 13 Feb 2017 13:49:44 +0000 Subject: Ordering is important on errbacks so add the cleanup func before creating an ObservableDeferred --- synapse/rest/client/transactions.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'synapse/rest/client') diff --git a/synapse/rest/client/transactions.py b/synapse/rest/client/transactions.py index 7b742ca7ae..fceca2edeb 100644 --- a/synapse/rest/client/transactions.py +++ b/synapse/rest/client/transactions.py @@ -87,19 +87,19 @@ class HttpTransactionCache(object): deferred = fn(*args, **kwargs) - # We don't add an errback to the raw deferred, so we ask ObservableDeferred - # to swallow the error. This is fine as the error will still be reported - # to the observers. - observable = ObservableDeferred(deferred, consumeErrors=True) - self.transactions[txn_key] = (observable, self.clock.time_msec()) - # if the request fails with a Twisted failure, remove it # from the transaction map. This is done to ensure that we don't # cache transient errors like rate-limiting errors, etc. def remove_from_map(err): self.transactions.pop(txn_key, None) return err - observable.addErrback(remove_from_map) + deferred.addErrback(remove_from_map) + + # We don't add any other errbacks to the raw deferred, so we ask + # ObservableDeferred to swallow the error. This is fine as the error will + # still be reported to the observers. + observable = ObservableDeferred(deferred, consumeErrors=True) + self.transactions[txn_key] = (observable, self.clock.time_msec()) return observable.observe() def _cleanup(self): -- cgit 1.4.1 From 355d62c499b63f337c484cca6704161a3a05da1d Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 14 Feb 2017 15:10:55 +0000 Subject: Make kick & ban reasons work We somehow specced APIs with reason strings, preserve the content in the events and even have the clients display them, but failed to actually pass the parameter through to the event content. --- synapse/rest/client/v1/room.py | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'synapse/rest/client') diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 2ebf5e59a0..728e3df0e3 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -609,6 +609,10 @@ class RoomMembershipRestServlet(ClientV1RestServlet): raise SynapseError(400, "Missing user_id key.") target = UserID.from_string(content["user_id"]) + event_content = None + if 'reason' in content and membership_action in ['kick', 'ban']: + event_content = {'reason': content['reason']} + yield self.handlers.room_member_handler.update_membership( requester=requester, target=target, @@ -616,6 +620,7 @@ class RoomMembershipRestServlet(ClientV1RestServlet): action=membership_action, txn_id=txn_id, third_party_signed=content.get("third_party_signed", None), + content=event_content, ) defer.returnValue((200, {})) -- cgit 1.4.1 From 17673404fbb1ca7b84efe7ca12955754bf366b09 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 20 Feb 2017 15:02:01 +0000 Subject: Remove unused param --- synapse/handlers/room.py | 2 +- synapse/rest/client/v1/room.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) (limited to 'synapse/rest/client') diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 73bc73a45e..99cb7db0db 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -356,7 +356,7 @@ class RoomCreationHandler(BaseHandler): class RoomContextHandler(BaseHandler): @defer.inlineCallbacks - def get_event_context(self, user, room_id, event_id, limit, is_guest): + def get_event_context(self, user, room_id, event_id, limit): """Retrieves events, pagination tokens and state around a given event in a room. diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 728e3df0e3..90242a6bac 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -505,7 +505,6 @@ class RoomEventContext(ClientV1RestServlet): room_id, event_id, limit, - requester.is_guest, ) if not results: -- cgit 1.4.1