From a3a0419e9c6e4fcba8d2e4a1bbe0e3515b7677cb Mon Sep 17 00:00:00 2001 From: Kislay Verma Date: Fri, 14 Mar 2025 14:00:11 +0530 Subject: [PATCH] popup_banners: Improve connection error banner. This commit makes the following changes: - Prevents the codepath in load_messages in message_fetch and get_events in server_events from interfering with each other. - The banner label now displays the time interval after which the connection will be retried. - The loading indicator now shows on the banner when the retry is in progress via the JS logic. Co-authored-by: Sayam Samal Fixes #33924. --- web/src/message_fetch.ts | 21 +++++++-- web/src/popup_banners.ts | 96 ++++++++++++++++++++++++++++++++++------ web/src/server_events.js | 11 +++-- 3 files changed, 104 insertions(+), 24 deletions(-) diff --git a/web/src/message_fetch.ts b/web/src/message_fetch.ts index edfcc3062e..5fa9440f49 100644 --- a/web/src/message_fetch.ts +++ b/web/src/message_fetch.ts @@ -371,6 +371,11 @@ export function get_parameters_for_message_fetch_api( return data; } +// We keep track of the load messages timeout at a module level +// to prevent multiple load messages requests from the error codepath +// from stacking up by cancelling the previous timeout. +let load_messages_timeout: ReturnType | undefined; + export function load_messages(opts: MessageFetchOptions, attempt = 1): void { const data = get_parameters_for_message_fetch_api(opts); let update_loading_indicator = @@ -393,7 +398,10 @@ export function load_messages(opts: MessageFetchOptions, attempt = 1): void { url: "/json/messages", data, success(raw_data) { - popup_banners.close_connection_error_popup_banner(true); + if (load_messages_timeout !== undefined) { + clearTimeout(load_messages_timeout); + } + popup_banners.close_connection_error_popup_banner("message_fetch"); const data = response_schema.parse(raw_data); get_messages_success(data, opts); }, @@ -402,7 +410,10 @@ export function load_messages(opts: MessageFetchOptions, attempt = 1): void { // We successfully reached the server, so hide the // connection error notice, even if the request failed // for other reasons. - popup_banners.close_connection_error_popup_banner(true); + if (load_messages_timeout !== undefined) { + clearTimeout(load_messages_timeout); + } + popup_banners.close_connection_error_popup_banner("message_fetch"); } if ( @@ -442,14 +453,16 @@ export function load_messages(opts: MessageFetchOptions, attempt = 1): void { return; } + const delay_secs = util.get_retry_backoff_seconds(xhr, attempt, true); popup_banners.open_connection_error_popup_banner({ + caller: "message_fetch", + retry_delay_secs: delay_secs, on_retry_callback() { load_messages(opts, attempt + 1); }, }); - const delay_secs = util.get_retry_backoff_seconds(xhr, attempt, true); - setTimeout(() => { + load_messages_timeout = setTimeout(() => { load_messages(opts, attempt + 1); }, delay_secs * 1000); }, diff --git a/web/src/popup_banners.ts b/web/src/popup_banners.ts index 14b01e0078..b970f506c1 100644 --- a/web/src/popup_banners.ts +++ b/web/src/popup_banners.ts @@ -5,6 +5,9 @@ import type {Banner} from "./banners.ts"; import * as buttons from "./buttons.ts"; import {$t} from "./i18n.ts"; +let retry_connection_interval: ReturnType | undefined; +let original_retry_delay_secs = 0; + function fade_out_popup_banner($banner: JQuery): void { $banner.addClass("fade-out"); // The delay is the same as the animation duration for fade-out. @@ -13,11 +16,25 @@ function fade_out_popup_banner($banner: JQuery): void { }, 300); } -const CONNECTION_ERROR_POPUP_BANNER: Banner = { +const get_connection_error_label = (retry_delay_secs: number): string => { + if (original_retry_delay_secs < 5) { + // When the retry delay is less than 5 seconds, we don't show the retry + // delay time in the banner, and instead just show "Retrying soon…" to + // constant flickering of the banner label for very short times. + return $t({defaultMessage: "Unable to connect to Zulip. Retrying soon…"}); + } + return $t( + { + defaultMessage: + "Unable to connect to Zulip. {retry_delay_secs, plural, one {Trying again in {retry_delay_secs} second…} other {Trying again in {retry_delay_secs} seconds…}}", + }, + {retry_delay_secs}, + ); +}; + +const connection_error_popup_banner = (retry_seconds: number): Banner => ({ intent: "danger", - label: $t({ - defaultMessage: "Unable to connect to Zulip. Retrying soon…", - }), + label: get_connection_error_label(retry_seconds), buttons: [ { attention: "quiet", @@ -27,6 +44,29 @@ const CONNECTION_ERROR_POPUP_BANNER: Banner = { ], close_button: true, custom_classes: "connection-error-banner popup-banner", +}); + +const update_connection_error_banner = ($banner: JQuery, retry_delay_secs: number): void => { + original_retry_delay_secs = retry_delay_secs; + if (retry_connection_interval !== undefined) { + clearInterval(retry_connection_interval); + } + const $banner_label = $banner.find(".banner-label"); + retry_connection_interval = setInterval(() => { + retry_delay_secs -= 1; + if (retry_delay_secs <= 0) { + // When the retry delay is over, stop the retry interval. + clearInterval(retry_connection_interval); + return; + } + if (retry_delay_secs <= 1) { + // One second before the retry, show the loading indicator to + // visually indicate that the retry sequence is being executed. + const $retry_connection_button = $banner.find(".retry-connection"); + buttons.show_button_loading_indicator($retry_connection_button); + } + $banner_label.text(get_connection_error_label(retry_delay_secs)); + }, 1000); }; // Show user a banner with a button to allow user to navigate @@ -74,13 +114,23 @@ export function close_found_missing_unreads_banner(): void { } export function open_connection_error_popup_banner(opts: { + caller: "server_events" | "message_fetch"; + retry_delay_secs: number; on_retry_callback: () => void; - is_get_events_error?: boolean; }): void { + opts.retry_delay_secs = Math.round(opts.retry_delay_secs); // If the banner is already open, don't open it again, and instead remove // the loading indicator on the retry button, if it was being shown. - const $banner = $("#popup_banners_wrapper").find(".connection-error-banner"); + let $banner = $("#popup_banners_wrapper").find(".connection-error-banner"); if ($banner.length > 0) { + if ($banner.attr("data-caller") !== opts.caller) { + // Only the original caller should be able to modify the banner. + // This prevents the interference between the server errors from + // get_events in web/src/server_events.js and the one from + // load_messages in web/src/message_fetch.ts. + return; + } + update_connection_error_banner($banner, opts.retry_delay_secs); const $retry_connection_button = $banner.find(".retry-connection"); if ($retry_connection_button.find(".button-loading-indicator").length > 0) { // Add some delay before hiding the loading indicator, to visually @@ -91,18 +141,30 @@ export function open_connection_error_popup_banner(opts: { } return; } - // Prevent the interference between the server errors from - // get_events in web/src/server_events.js and the one from - // load_messages in web/src/message_fetch.ts. - if (opts.is_get_events_error) { - CONNECTION_ERROR_POPUP_BANNER.custom_classes += " get-events-error"; + + banners.append( + connection_error_popup_banner(opts.retry_delay_secs), + $("#popup_banners_wrapper"), + ); + + $banner = $("#popup_banners_wrapper").find(".connection-error-banner"); + if (opts.caller === "server_events") { + $banner.attr("data-caller", "server_events"); + } else if (opts.caller === "message_fetch") { + $banner.attr("data-caller", "message_fetch"); } - banners.append(CONNECTION_ERROR_POPUP_BANNER, $("#popup_banners_wrapper")); + + update_connection_error_banner($banner, opts.retry_delay_secs); $("#popup_banners_wrapper").on("click", ".retry-connection", function (this: HTMLElement, e) { e.preventDefault(); e.stopPropagation(); + const $banner = $(this).closest(".banner"); + $banner + .find(".banner-label") + .text($t({defaultMessage: "Unable to connect to Zulip. Retrying now…"})); + const $button = $(this); // If the loading indicator is already being shown, this logic @@ -118,14 +180,20 @@ export function open_connection_error_popup_banner(opts: { }); } -export function close_connection_error_popup_banner(check_if_get_events_error = false): void { +export function close_connection_error_popup_banner( + caller: "server_events" | "message_fetch", +): void { const $banner = $("#popup_banners_wrapper").find(".connection-error-banner"); if ($banner.length === 0) { return; } - if (check_if_get_events_error && $banner.hasClass("get-events-error")) { + if ($banner.attr("data-caller") !== caller) { + // Only the original caller should be able to modify the banner. return; } + if (retry_connection_interval !== undefined) { + clearInterval(retry_connection_interval); + } fade_out_popup_banner($banner); } diff --git a/web/src/server_events.js b/web/src/server_events.js index 09dbe088b9..10f03dc783 100644 --- a/web/src/server_events.js +++ b/web/src/server_events.js @@ -192,7 +192,7 @@ function get_events({dont_block = false} = {}) { try { get_events_xhr = undefined; get_events_failures = 0; - popup_banners.close_connection_error_popup_banner(); + popup_banners.close_connection_error_popup_banner("server_events"); get_events_success(data.events); } catch (error) { @@ -201,6 +201,7 @@ function get_events({dont_block = false} = {}) { get_events_timeout = setTimeout(get_events, 0); }, error(xhr, error_type) { + const retry_delay_secs = util.get_retry_backoff_seconds(xhr, get_events_failures); try { get_events_xhr = undefined; // If we're old enough that our message queue has been @@ -220,26 +221,24 @@ function get_events({dont_block = false} = {}) { } else if (error_type === "timeout") { // Retry indefinitely on timeout. get_events_failures = 0; - popup_banners.close_connection_error_popup_banner(); + popup_banners.close_connection_error_popup_banner("server_events"); } else { get_events_failures += 1; } if (get_events_failures >= 8) { popup_banners.open_connection_error_popup_banner({ + caller: "server_events", + retry_delay_secs, on_retry_callback() { restart_get_events({dont_block: true}); }, - is_get_events_error: true, }); - } else { - popup_banners.close_connection_error_popup_banner(); } } catch (error) { blueslip.error("Failed to handle get_events error", undefined, error); } - const retry_delay_secs = util.get_retry_backoff_seconds(xhr, get_events_failures); get_events_timeout = setTimeout(get_events, retry_delay_secs * 1000); }, });