From e3960c22bea2b92df24cdfaed66a5a48dc6faa94 Mon Sep 17 00:00:00 2001
From: Tim Abbott <tabbott@zulip.com>
Date: Fri, 2 Feb 2024 13:25:43 -0800
Subject: [PATCH] web: Respect rate-limiting headers in main APIs.

Previously, these endpoints just did exponential backoff, without
looking at the rate-limiting headers returned by the server, resulting
in requests that the client could have been certain would fail with an
additional rate-limiting error.

Fix this by using the maximum of the existing exponential backoff with
the value returned by the rate-limiting header.

Fixes #28807.
---
 web/src/message_fetch.js | 26 ++++++++++++++++++++------
 web/src/server_events.js | 17 +++++++++++++++--
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/web/src/message_fetch.js b/web/src/message_fetch.js
index 88067fbdac..d91205d73e 100644
--- a/web/src/message_fetch.js
+++ b/web/src/message_fetch.js
@@ -370,15 +370,29 @@ export function load_messages(opts, attempt = 1) {
                 return;
             }
 
-            // Backoff on retries, with full jitter: up to 2s, 4s, 8s, 16s, 32s
-            let delay = Math.random() * 2 ** attempt * 2000;
-            if (attempt >= 5) {
-                delay = 30000;
-            }
             ui_report.show_error($("#connection-error"));
+
+            // We need to respect the server's rate-limiting headers, but beyond
+            // that, we also want to avoid contributing to a thundering herd if
+            // the server is giving us 500s/502s.
+            //
+            // So we do the maximum of the retry-after header and an exponential
+            // backoff with full jitter: up to 2s, 4s, 8s, 16s, 32s
+            let backoff_delay_secs = Math.random() * 2 ** attempt * 2;
+            if (attempt >= 5) {
+                backoff_delay_secs = 30;
+            }
+            let rate_limit_delay_secs = 0;
+            if (xhr.status === 429 && xhr.responseJSON?.code === "RATE_LIMIT_HIT") {
+                // Add a bit of jitter to the required delay suggested by the
+                // server, because we may be racing with other copies of the web
+                // app.
+                rate_limit_delay_secs = xhr.responseJSON["retry-after"] + Math.random() * 0.5;
+            }
+            const delay_secs = Math.max(backoff_delay_secs, rate_limit_delay_secs);
             setTimeout(() => {
                 load_messages(opts, attempt + 1);
-            }, delay);
+            }, delay_secs * 1000);
         },
     });
 }
diff --git a/web/src/server_events.js b/web/src/server_events.js
index b319c7e73e..cd5d7587e9 100644
--- a/web/src/server_events.js
+++ b/web/src/server_events.js
@@ -239,8 +239,21 @@ function get_events({dont_block = false} = {}) {
             } catch (error) {
                 blueslip.error("Failed to handle get_events error", undefined, error);
             }
-            const retry_sec = Math.min(90, Math.exp(get_events_failures / 2));
-            get_events_timeout = setTimeout(get_events, retry_sec * 1000);
+
+            // We need to respect the server's rate-limiting headers, but beyond
+            // that, we also want to avoid contributing to a thundering herd if
+            // the server is giving us 500s/502s.
+            const backoff_delay_secs = Math.min(90, Math.exp(get_events_failures / 2));
+            let rate_limit_delay_secs = 0;
+            if (xhr.status === 429 && xhr.responseJSON?.code === "RATE_LIMIT_HIT") {
+                // Add a bit of jitter to the required delay suggested
+                // by the server, because we may be racing with other
+                // copies of the web app.
+                rate_limit_delay_secs = xhr.responseJSON["retry-after"] + Math.random() * 0.5;
+            }
+
+            const retry_delay_secs = Math.max(backoff_delay_secs, rate_limit_delay_secs);
+            get_events_timeout = setTimeout(get_events, retry_delay_secs * 1000);
         },
     });
 }