FIX: Don't spam presence requests when getting 429 (#20084)

The presence service would retry `/presence/update` requests every second (or immediately in tests) in case where server returns 429 (rate limit) errors. That could lead to infinite spamming (until user refreshed tab/tabs)

Co-authored-by: David Taylor <david@taylorhq.com>
This commit is contained in:
Jarek Radosz 2023-01-31 09:49:23 +01:00
parent 78fe2656b4
commit df70c8bf1c
2 changed files with 35 additions and 3 deletions

View File

@ -16,7 +16,7 @@ import { isTesting } from "discourse-common/config/environment";
import getURL from "discourse-common/lib/get-url"; import getURL from "discourse-common/lib/get-url";
const PRESENCE_INTERVAL_S = 30; const PRESENCE_INTERVAL_S = 30;
const PRESENCE_DEBOUNCE_MS = isTesting() ? 0 : 500; const DEFAULT_PRESENCE_DEBOUNCE_MS = isTesting() ? 0 : 500;
const PRESENCE_THROTTLE_MS = isTesting() ? 0 : 1000; const PRESENCE_THROTTLE_MS = isTesting() ? 0 : 1000;
const PRESENCE_GET_RETRY_MS = 5000; const PRESENCE_GET_RETRY_MS = 5000;
@ -251,6 +251,8 @@ class PresenceChannelState extends EmberObject.extend(Evented) {
} }
export default class PresenceService extends Service { export default class PresenceService extends Service {
_presenceDebounceMs = DEFAULT_PRESENCE_DEBOUNCE_MS;
init() { init() {
super.init(...arguments); super.init(...arguments);
this._queuedEvents = []; this._queuedEvents = [];
@ -276,6 +278,7 @@ export default class PresenceService extends Service {
super.willDestroy(...arguments); super.willDestroy(...arguments);
window.removeEventListener("beforeunload", this._beaconLeaveAll); window.removeEventListener("beforeunload", this._beaconLeaveAll);
removeOnPresenceChange(this._throttledUpdateServer); removeOnPresenceChange(this._throttledUpdateServer);
cancel(this._debounceTimer);
} }
// Get a PresenceChannel object representing a single channel // Get a PresenceChannel object representing a single channel
@ -543,11 +546,15 @@ export default class PresenceService extends Service {
e.promiseProxy.resolve(); e.promiseProxy.resolve();
} }
}); });
this._presenceDebounceMs = DEFAULT_PRESENCE_DEBOUNCE_MS;
} catch (e) { } catch (e) {
// Put the failed events back in the queue for next time // Put the failed events back in the queue for next time
this._queuedEvents.unshift(...queue); this._queuedEvents.unshift(...queue);
if (e.jqXHR?.status === 429) { if (e.jqXHR?.status === 429) {
// Rate limited. No need to raise, we'll try again later // Rate limited
const waitSeconds = e.jqXHR.responseJSON?.extras?.wait_seconds || 10;
this._presenceDebounceMs = waitSeconds * 1000;
} else { } else {
throw e; throw e;
} }
@ -585,7 +592,12 @@ export default class PresenceService extends Service {
return; return;
} else if (this._queuedEvents.length > 0) { } else if (this._queuedEvents.length > 0) {
this._cancelTimer(); this._cancelTimer();
debounce(this, this._throttledUpdateServer, PRESENCE_DEBOUNCE_MS); cancel(this._debounceTimer);
this._debounceTimer = debounce(
this,
this._throttledUpdateServer,
this._presenceDebounceMs
);
} else if ( } else if (
!this._nextUpdateTimer && !this._nextUpdateTimer &&
this._presentChannels.length > 0 && this._presentChannels.length > 0 &&

View File

@ -477,4 +477,24 @@ module("Unit | Service | presence | entering and leaving", function (hooks) {
"skips sending empty updates to the server" "skips sending empty updates to the server"
); );
}); });
test("don't spam requests when server returns 429", function (assert) {
const done = assert.async();
let requestCount = 0;
pretender.post("/presence/update", async () => {
requestCount++;
return response(429, { extras: { wait_seconds: 2 } });
});
const presenceService = getOwner(this).lookup("service:presence");
presenceService.currentUser = currentUser();
const channel = presenceService.getChannel("/test/ch1");
setTimeout(function () {
assert.strictEqual(requestCount, 1);
done();
}, 500);
channel.enter();
});
}); });