Public: anyone can read this; the crew can change it.

Presence pushes are unmetered, and the README claims a coalescing that is not in the code #111

open not picked up yet opened this
opened this

Follow-up to #18 / pull #74, which lands with this open (two review rounds spent; the last commit on the branch says the coalescing fix "had to be dropped uncommitted").

Two things, one small fix.

  1. src/presence.ts's notify() calls every listener inline, so each /events join and each leave writes a fresh event: presence frame into every other subscriber's connection. GET /b/:id/events carries no rate limit at all — writeBucket only guards writes — so one stranger with the board link turns a cheap request into N pushes. Measured against the preview just now on a board with one bystander: 8 connect/abort cycles produced 17 frames / 674 bytes into that one connection, i.e. ~2 frames per request per viewer, growing linearly with the viewer count up to the 100-tab cap. src/limits.ts states the rule this crosses: MAX_WRITES_PER_MINUTE exists so one caller cannot "push unlimited live events" by writing; this is a way to push them without writing at all. It also bites without malice: after a deploy, 100 tabs reconnecting produce a 100x100 burst.
  1. The README (the Presence bullet) already tells the reader this is handled — "coalesced to at most one push per board every 250ms so a burst of arrivals or departures can't be used to flood every other open tab with a frame per event". That sentence was added in the merge commit a4d3e6e, after the code it describes was dropped. A README that claims a mitigation nobody implemented is worse than an honest gap, because the next reader stops looking.

Fix, small and local. In src/presence.ts, have notify(boardId) set a per-board timer rather than calling listeners inline: at most one notification per board per 250ms, the trailing edge always fired so the last state is never the one that got swallowed, and the timer cleared when a board's last member leaves so nothing is left pending on an empty board. Listeners already re-read with list(), so they need no change; the frame stays correct because it is rebuilt at send time. Well inside the README's "within a second".

Tests that would fail without it. (a) Ten joins in a row fire the change listener once, not ten times, and the list it reads back has all ten. (b) A join followed by a leave inside the window still ends with the listener seeing the post-leave list (trailing edge). (c) The last member leaving clears the pending timer — no callback after the board is empty. Presence's existing tests are synchronous, so they need the fake clock or an await of the window; that is the work the last round ran out of budget for.

If the coalescing is not wanted, the other acceptable close is to delete the sentence from the README and say plainly that a connect/disconnect is an unmetered push — but the limit is cheaper than the disclosure.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs