From 1c88115533c8d1b0abae04b7b6bf9e04e67da523 Mon Sep 17 00:00:00 2001 From: "neiljp (Neil Pilgrim)" Date: Fri, 17 Apr 2020 21:56:13 -0700 Subject: [PATCH 1/3] api: Use exponential backoff in call_on_each_event. Previously we paused 1s after each failure. --- zulip/zulip/__init__.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/zulip/zulip/__init__.py b/zulip/zulip/__init__.py index a363955ad..0a77c10ba 100644 --- a/zulip/zulip/__init__.py +++ b/zulip/zulip/__init__.py @@ -619,7 +619,13 @@ def do_register() -> Tuple[str, int]: # Make long-polling requests with `get_events`. Once a request # has received an answer, pass it to the callback and before # making a new long-polling request. - while True: + # NOTE: Back off exponentially to cover against potential bugs in this + # library causing a DoS attack against a server when getting errors + # (explicit values listed for clarity) + backoff = RandomExponentialBackoff(maximum_retries=10, + timeout_success_equivalent=300, + delay_cap=90) + while backoff.keep_going(): if queue_id is None: (queue_id, last_event_id) = do_register() @@ -649,12 +655,11 @@ def do_register() -> Tuple[str, int]: # # Reset queue_id to register a new event queue. queue_id = None - # Add a pause here to cover against potential bugs in this library - # causing a DoS attack against a server when getting errors. - # TODO: Make this back off exponentially. - time.sleep(1) + + backoff.fail() continue + backoff.succeed() for event in res['events']: last_event_id = max(last_event_id, int(event['id'])) callback(event) From 292d320af171496b882f71f1e0c7691705f737d5 Mon Sep 17 00:00:00 2001 From: "neiljp (Neil Pilgrim)" Date: Sun, 26 Apr 2020 01:07:53 -0700 Subject: [PATCH 2/3] api: Extend backoff for register inside call_on_each_event. --- zulip/zulip/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/zulip/zulip/__init__.py b/zulip/zulip/__init__.py index 0a77c10ba..17956d931 100644 --- a/zulip/zulip/__init__.py +++ b/zulip/zulip/__init__.py @@ -601,8 +601,8 @@ def call_on_each_event( if narrow is None: narrow = [] - def do_register() -> Tuple[str, int]: - while True: + def do_register(backoff: RandomExponentialBackoff) -> Tuple[str, int]: + while backoff.keep_going(): if event_types is None: res = self.register() else: @@ -611,8 +611,9 @@ def do_register() -> Tuple[str, int]: if 'error' in res['result']: if self.verbose: print("Server returned error:\n%s" % res['msg']) - time.sleep(1) + backoff.fail() else: + backoff.succeed() return (res['queue_id'], res['last_event_id']) queue_id = None @@ -627,7 +628,7 @@ def do_register() -> Tuple[str, int]: delay_cap=90) while backoff.keep_going(): if queue_id is None: - (queue_id, last_event_id) = do_register() + (queue_id, last_event_id) = do_register(backoff) res = self.get_events(queue_id=queue_id, last_event_id=last_event_id) if 'error' in res['result']: From dc4404ffc7a4cc19e979b9bb308fcda119633d37 Mon Sep 17 00:00:00 2001 From: "neiljp (Neil Pilgrim)" Date: Sun, 26 Apr 2020 01:26:35 -0700 Subject: [PATCH 3/3] api: Inline nested do_register function in call_on_each_event. --- zulip/zulip/__init__.py | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/zulip/zulip/__init__.py b/zulip/zulip/__init__.py index 17956d931..3791e50ab 100644 --- a/zulip/zulip/__init__.py +++ b/zulip/zulip/__init__.py @@ -601,8 +601,16 @@ def call_on_each_event( if narrow is None: narrow = [] - def do_register(backoff: RandomExponentialBackoff) -> Tuple[str, int]: - while backoff.keep_going(): + queue_id = None + # NOTE: Back off exponentially to cover against potential bugs in this + # library causing a DoS attack against a server when getting errors + # (explicit values listed for clarity) + backoff = RandomExponentialBackoff(maximum_retries=10, + timeout_success_equivalent=300, + delay_cap=90) + while backoff.keep_going(): + # Ensure event queue exists (or continues to do so) + if queue_id is None: if event_types is None: res = self.register() else: @@ -612,24 +620,14 @@ def do_register(backoff: RandomExponentialBackoff) -> Tuple[str, int]: if self.verbose: print("Server returned error:\n%s" % res['msg']) backoff.fail() + continue else: backoff.succeed() - return (res['queue_id'], res['last_event_id']) - - queue_id = None - # Make long-polling requests with `get_events`. Once a request - # has received an answer, pass it to the callback and before - # making a new long-polling request. - # NOTE: Back off exponentially to cover against potential bugs in this - # library causing a DoS attack against a server when getting errors - # (explicit values listed for clarity) - backoff = RandomExponentialBackoff(maximum_retries=10, - timeout_success_equivalent=300, - delay_cap=90) - while backoff.keep_going(): - if queue_id is None: - (queue_id, last_event_id) = do_register(backoff) + queue_id, last_event_id = res['queue_id'], res['last_event_id'] + # Make long-polling requests with `get_events`. Once a request + # has received an answer, pass it to the callback and before + # making a new long-polling request. res = self.get_events(queue_id=queue_id, last_event_id=last_event_id) if 'error' in res['result']: if res["result"] == "http-error":