From dbf19fe8d7d7b5af0282a771646abd740fdf1c87 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Mon, 22 Mar 2021 12:59:50 +0000 Subject: [PATCH] refactor: Extract watchdog module. We now have 100% code coverage on this somewhat fiddly code. We also break activity's dependency on server_events. --- docs/subsystems/hashchange-system.md | 4 +- frontend_tests/node_tests/activity.js | 6 +-- frontend_tests/node_tests/watchdog.js | 63 +++++++++++++++++++++++++++ static/js/activity.js | 4 +- static/js/server_events.js | 21 +-------- static/js/watchdog.js | 34 +++++++++++++++ 6 files changed, 106 insertions(+), 26 deletions(-) create mode 100644 frontend_tests/node_tests/watchdog.js create mode 100644 static/js/watchdog.js diff --git a/docs/subsystems/hashchange-system.md b/docs/subsystems/hashchange-system.md index 1608d30fcc..89bab15caa 100644 --- a/docs/subsystems/hashchange-system.md +++ b/docs/subsystems/hashchange-system.md @@ -86,8 +86,8 @@ reload itself: garbage-collected by the server, meaning the browser can no longer get real-time updates altogether. In this case, the browser auto-reloads immediately in order to reconnect. We have coded an - unsuspend trigger (based on some clever time logic) that ensures we - check immediately when a client unsuspends; grep for `unsuspend` to + unsuspend callback (based on some clever time logic) that ensures we + check immediately when a client unsuspends; grep for `watchdog` to see the code. * If a new version of the server has been deployed, we want to reload the browser so that it will start running the latest code. However, diff --git a/frontend_tests/node_tests/activity.js b/frontend_tests/node_tests/activity.js index 4f6c540dfc..a2de1c0496 100644 --- a/frontend_tests/node_tests/activity.js +++ b/frontend_tests/node_tests/activity.js @@ -51,12 +51,12 @@ mock_esm("../../static/js/resize", { mock_esm("../../static/js/scroll_util", { scroll_element_into_container: () => {}, }); -mock_esm("../../static/js/server_events", { - check_for_unsuspend() {}, -}); mock_esm("../../static/js/stream_popover", { show_streamlist_sidebar() {}, }); +mock_esm("../../static/js/watchdog", { + check_for_unsuspend() {}, +}); set_global("document", _document); const huddle_data = zrequire("huddle_data"); diff --git a/frontend_tests/node_tests/watchdog.js b/frontend_tests/node_tests/watchdog.js new file mode 100644 index 0000000000..376481a54e --- /dev/null +++ b/frontend_tests/node_tests/watchdog.js @@ -0,0 +1,63 @@ +"use strict"; + +const {strict: assert} = require("assert"); + +const MockDate = require("mockdate"); + +const {set_global, zrequire} = require("../zjsunit/namespace"); +const {run_test} = require("../zjsunit/test"); + +let time = 0; +let checker; +MockDate.set(time); + +function advance_secs(secs) { + time += secs * 1000; + MockDate.set(time); +} + +set_global("setInterval", (f, interval) => { + checker = f; + assert.equal(interval, 5000); +}); + +const watchdog = zrequire("watchdog"); + +run_test("basics", () => { + // Test without callbacks first. + checker(); + advance_secs(5); + checker(); + + let num_times_called_back = 0; + + function callback() { + num_times_called_back += 1; + } + + watchdog.on_unsuspend(callback); + + // Simulate healthy operation. + advance_secs(5); + checker(); + assert.equal(num_times_called_back, 0); + + // Simulate machine going to sleep. + advance_secs(21); + checker(); + assert.equal(num_times_called_back, 1); + + // Simulate healthy operations resume, and + // explicitly call check_for_unsuspend. + num_times_called_back = 0; + advance_secs(5); + watchdog.check_for_unsuspend(); + assert.equal(num_times_called_back, 0); + + // Simulate another suspension. + advance_secs(21); + watchdog.check_for_unsuspend(); + assert.equal(num_times_called_back, 1); +}); + +MockDate.reset(); diff --git a/static/js/activity.js b/static/js/activity.js index 50a230f42f..85dc645bcb 100644 --- a/static/js/activity.js +++ b/static/js/activity.js @@ -12,9 +12,9 @@ import * as people from "./people"; import * as pm_list from "./pm_list"; import * as popovers from "./popovers"; import * as presence from "./presence"; -import * as server_events from "./server_events"; import {UserSearch} from "./user_search"; import * as user_status from "./user_status"; +import * as watchdog from "./watchdog"; export let user_cursor; export let user_filter; @@ -195,7 +195,7 @@ export function send_presence_to_server(want_redraw) { return; } - server_events.check_for_unsuspend(); + watchdog.check_for_unsuspend(); channel.post({ url: "/json/users/me/presence", diff --git a/static/js/server_events.js b/static/js/server_events.js index d3f112eb9f..63607430ec 100644 --- a/static/js/server_events.js +++ b/static/js/server_events.js @@ -11,6 +11,7 @@ import * as reload_state from "./reload_state"; import * as sent_messages from "./sent_messages"; import * as server_events_dispatch from "./server_events_dispatch"; import * as ui_report from "./ui_report"; +import * as watchdog from "./watchdog"; // Docs: https://zulip.readthedocs.io/en/latest/subsystems/events-system.html @@ -302,26 +303,8 @@ export function home_view_loaded() { $(document).trigger("home_view_loaded.zulip"); } -let watchdog_time = Date.now(); - -export function check_for_unsuspend() { - const new_time = Date.now(); - if (new_time - watchdog_time > 20000) { - // 20 seconds. - // Defensively reset watchdog_time here in case there's an - // exception in one of the event handlers - watchdog_time = new_time; - // Our app's JS wasn't running, which probably means the machine was - // asleep. - $(document).trigger("unsuspend"); - } - watchdog_time = new_time; -} - -setInterval(check_for_unsuspend, 5000); - export function initialize() { - $(document).on("unsuspend", () => { + watchdog.on_unsuspend(() => { // Immediately poll for new events on unsuspend blueslip.log("Restarting get_events due to unsuspend"); get_events_failures = 0; diff --git a/static/js/watchdog.js b/static/js/watchdog.js new file mode 100644 index 0000000000..f2d427c33f --- /dev/null +++ b/static/js/watchdog.js @@ -0,0 +1,34 @@ +const unsuspend_callbacks = []; +let watchdog_time = Date.now(); + +/* + Our watchdog code checks every 5 seconds to make sure that we + haven't gone 20 seconds since the last "5-second-ago" check. + This sounds confusing, but it is just is a way to detect that + the machine has gone to sleep. + + When we detect the condition we call back to server_events code + to reset ourselves accordingly. +*/ + +export function check_for_unsuspend() { + const new_time = Date.now(); + if (new_time - watchdog_time > 20000) { + // 20 seconds. + // Defensively reset watchdog_time here in case there's an + // exception in one of the event handlers + watchdog_time = new_time; + // Our app's JS wasn't running, which probably means the machine was + // asleep. + for (const callback of unsuspend_callbacks) { + callback(); + } + } + watchdog_time = new_time; +} + +export function on_unsuspend(f) { + unsuspend_callbacks.push(f); +} + +setInterval(check_for_unsuspend, 5000);