Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 88 additions & 34 deletions extensions/realtime/js/src/forum/extend/Application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,42 +133,93 @@ export default function () {
}
};

app.websocket = new Pusher(wsKey, pusherOptions);
setupChannels(app.websocket);

// iOS browsers (all WebKit) silently drop WebSocket connections when
// the tab is backgrounded or the device sleeps, without firing `close`
// — pusher-js's built-in recovery never triggers, so realtime updates
// go missing until the page is reloaded. iOS also bfcaches pages on
// app-switch, which restores via `pageshow` (persisted=true) and does
// WebKit silently drops WebSocket connections when the tab is
// backgrounded or the device sleeps, often without firing `close` —
// pusher-js's built-in recovery never triggers (or triggers only after
// its foreground-only activity/pong timers finally elapse), so realtime
// updates go missing until the page is reloaded. iOS also bfcaches pages
// on app-switch, which restores via `pageshow` (persisted=true) and does
// NOT fire `visibilitychange` on return. We therefore hook both events.
//
// The visibilitychange path is gated on `isIOS()`: desktop browsers
// (and Android) maintain the WebSocket fine across tab backgrounding
// and don't need a forced reconnect, which would otherwise cause an
// unnecessary discussion-list refetch on every tab-switch return.
// The visibilitychange path reconnects when on iOS (where backgrounding
// kills the socket every time — see #4590/#4654) OR when the connection
// is demonstrably unhealthy: pusher-js already reports a non-connected
// state, or no protocol frame (events, pusher:pong, ...) has arrived
// within the activity window — the zombie case where `state` still
// claims 'connected'. Desktop Safari suspends hidden pages just like
// iOS, so it routinely lands in that zombie case (#4717). Healthy
// desktop tabs keep receiving pongs every ≤30s even while hidden, so a
// plain tab switch still triggers no spurious refetch (#4662).
//
// `forceReconnect` constructs a fresh Pusher instance rather than
// calling `connect()` on the existing one. pusher-js 7.6's default
// strategy enforces a `lives: 2` budget on its WebSocket transport;
// every iOS-initiated 1006 close decrements the budget and after the
// second backgrounding the strategy reports unsupported, so every
// subsequent `connect()` transitions straight to `'failed'`. A fresh
// Pusher comes with a fresh strategy tree and a full `livesLeft`,
// making the recovery survive arbitrarily many backgrounding cycles.
//
// After reconnecting, Pusher has no server-side buffering for events
// that fired while the socket was dead — we refresh the visible
// discussions list once the new connection reports `'connected'` so the
// UI catches up on missed activity. Refresh is gated on the
// `'connected'` event (not fired immediately after `connect()`) because
// an immediate Mithril redraw races with pusher-js's channel
// resubscription and can leave the client receiving no further push
// events.
// every silent 1006 close decrements the budget and after the second
// one the strategy reports unsupported, so every subsequent `connect()`
// transitions straight to `'failed'`. A fresh Pusher comes with a fresh
// strategy tree and a full `livesLeft`, making the recovery survive
// arbitrarily many backgrounding cycles.
//
// See flarum/framework#4588 and #4597.
// See flarum/framework#4588, #4597 and #4717.
const RECONNECT_HIDDEN_THRESHOLD_MS = 5_000;
// A healthy connection sees protocol traffic at least every
// `activity_timeout` (30s, advertised by the server) plus the pong wait.
const STALE_CONNECTION_THRESHOLD_MS = 65_000;

let hiddenSince: number | null = null;
let lastFrameAt = Date.now();
let everConnected = false;

// Refresh the data that realtime events would have kept current. Pusher
// has no server-side buffering: anything that fired while the socket was
// down is lost, so on reconnect the UI has to catch up by refetching.
const catchUp = (): void => {
(app as any).discussions?.refresh?.();

const discussion = app.current.get('discussion');
const stream = app.current.get('stream');

if (discussion && stream) {
// Capture BEFORE the refetch grows postIds: missed events never
// reached the store, so right now viewingEnd() still reflects the
// last synced state. Afterwards its 1-post drift tolerance could no
// longer tell "was at the end, several posts arrived while
// disconnected" apart from "viewing an interior window" (#4717).
const wasViewingEnd: boolean = !!stream.viewingEnd();

app.store.find('discussions', discussion.id() as string).then(() => {
if (wasViewingEnd) {
stream.syncEnd().then((): void => m.redraw());
} else {
m.redraw();
}
});
}
};

// Track liveness and reconnects. Applied to the initial Pusher instance
// and to every fresh instance `forceReconnect` builds.
const watchConnection = (websocket: Pusher): void => {
websocket.bind_global((): void => {
lastFrameAt = Date.now();
});

websocket.connection.bind('state_change', (states: { previous: string; current: string }) => {
if (states.current !== 'connected') return;

const isReconnect = everConnected;
everConnected = true;
lastFrameAt = Date.now();

// Catch up on every effective reconnect — pusher-js's own recovery
// as well as our forced ones. Gated on the transition into
// `'connected'` (not run right after `connect()`) because an
// immediate Mithril redraw races with pusher-js's channel
// resubscription and can leave the client receiving no further push
// events (see #4590).
if (isReconnect) catchUp();
});
};

const forceReconnect = (): void => {
const previous = app.websocket;
Expand All @@ -177,16 +228,15 @@ export default function () {
const fresh = new Pusher(wsKey, pusherOptions);
app.websocket = fresh;

const onReconnected = (): void => {
fresh.connection.unbind('connected', onReconnected);
(app as any).discussions?.refresh?.();
};
fresh.connection.bind('connected', onReconnected);

watchConnection(fresh);
setupChannels(fresh);
RealtimeState.notifyChannelsReconnected();
};

app.websocket = new Pusher(wsKey, pusherOptions);
watchConnection(app.websocket);
setupChannels(app.websocket);

// Application.mount() runs once per page load, so these listeners are
// installed once and live for the lifetime of the page — no teardown needed.
document.addEventListener('visibilitychange', () => {
Expand All @@ -197,7 +247,11 @@ export default function () {
if (hiddenSince === null) return;
const wasHiddenFor = Date.now() - hiddenSince;
hiddenSince = null;
if (wasHiddenFor > RECONNECT_HIDDEN_THRESHOLD_MS && isIOS()) {
if (wasHiddenFor <= RECONNECT_HIDDEN_THRESHOLD_MS) return;

const unhealthy = app.websocket?.connection.state !== 'connected' || Date.now() - lastFrameAt > STALE_CONNECTION_THRESHOLD_MS;

if (isIOS() || unhealthy) {
forceReconnect();
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,24 @@ const CORE_REVISED_EVENT = 'revisedEvent';
export default function (): void {
extend(DiscussionPage.prototype, 'oninit', function (this: any) {
this.websocketEventPosted = (data: unknown): void => {
// Capture BEFORE the payload (and the refetch below) updates postIds:
// once new posts are in the store, viewingEnd()'s 1-post drift
// tolerance can no longer tell "was at the end, several posts arrived
// at once" apart from "viewing an interior window" — so a single
// missed event would permanently disable live-append (#4717).
const wasViewingEnd: boolean = !!this.stream?.viewingEnd();

const discussion = app.store.pushPayload(data as Parameters<typeof app.store.pushPayload>[0]) as any;

if (discussion.id() === this.discussion?.id() && this.stream) {
const oldCount: number = this.discussion.commentCount();

app.store.find('discussions', this.discussion.id()).then(() => {
this.stream.update().then((): void => m.redraw());
if (wasViewingEnd) {
this.stream.syncEnd().then((): void => m.redraw());
} else {
m.redraw();
}

if (!document.hasFocus()) {
app.setTitleCount(Math.max(0, this.discussion.commentCount() - oldCount));
Expand All @@ -30,11 +41,19 @@ export default function (): void {
};

this.websocketEventStreamUpdate = (data: unknown): void => {
// Same pre-payload capture as websocketEventPosted (#4717) — event
// posts (sticky/lock/rename) move postIds too.
const wasViewingEnd: boolean = !!this.stream?.viewingEnd();

const discussion = app.store.pushPayload(data as Parameters<typeof app.store.pushPayload>[0]) as any;

if (discussion.id() === this.discussion?.id() && this.stream) {
app.store.find('discussions', this.discussion.id()).then(() => {
this.stream.update().then((): void => m.redraw());
if (wasViewingEnd) {
this.stream.syncEnd().then((): void => m.redraw());
} else {
m.redraw();
}
});
}
};
Expand Down
18 changes: 18 additions & 0 deletions framework/core/js/src/forum/states/PostStreamState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,28 @@ export default class PostStreamState {
/**
* Update the stream so that it loads and includes the latest posts in the
* discussion, if the end is being viewed.
*
* Note: viewingEnd() tolerates a drift of at most one post, so this is only
* suitable when the store has gained a single new post since the window was
* last synced (the live-event case). When several posts land in the store
* at once — e.g. they were missed while a realtime connection was down —
* capture viewingEnd() BEFORE pushing the new payloads and call syncEnd()
* directly instead.
*/
update() {
if (!this.viewingEnd()) return Promise.resolve();

return this.syncEnd();
}

/**
* Load through to the latest post in the discussion, expanding the visible
* window to the end of the stream. Unlike update(), this places no bound on
* how far the window has fallen behind — the caller is responsible for
* ensuring the end was being viewed before the posts being caught up on
* entered the store.
*/
syncEnd(): Promise<Post[]> {
this.visibleEnd = this.count();

return this.loadRange(this.visibleStart, this.visibleEnd);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,51 @@ describe('PostStreamState', () => {
expect(nearPage(discussion)).toEqual([]);
});
});

// Regression tests for #4717 — catching up after a missed-event gap.
//
// update() guards on viewingEnd(), which tolerates a drift of at most one
// post. That covers the live-event case (exactly one new post since the
// last sync), but once ≥2 posts have entered the store — e.g. one was
// missed while a realtime connection was down and the next live event
// refetched the discussion — every update() call returns early and
// live-append is permanently dead. syncEnd() is the unbounded variant that
// realtime catch-up paths call after capturing end-ness up front.
describe('update() / syncEnd() after a missed-event gap (#4717)', () => {
test('update() appends a single new post when viewing the end (live-event path)', async () => {
const { discussion, postIds } = seedStore(5, [0, 1, 2, 3, 4]);
// Window synced through post 4 of 5: one post arrived since.
const state = new PostStreamState(discussion, postsFor(postIds.slice(0, 4)));

await state.update();

expect((state as any).visibleEnd).toBe(5);
expect(state.posts()).toHaveLength(5);
expect(state.posts().every((p) => p !== null)).toBe(true);
});

test('update() refuses once the window is two or more posts behind (locks the guard semantics)', async () => {
const { discussion, postIds } = seedStore(6, [0, 1, 2, 3, 4, 5]);
// Window synced through post 4, but TWO posts entered the store since
// (one missed while disconnected + one live): bookkeeping alone cannot
// tell this apart from viewing an interior window, so update() must
// not append — callers with better knowledge use syncEnd().
const state = new PostStreamState(discussion, postsFor(postIds.slice(0, 4)));

await state.update();

expect((state as any).visibleEnd).toBe(4);
});

test('syncEnd() loads through to the latest post no matter how far behind the window is', async () => {
const { discussion, postIds } = seedStore(8, [0, 1, 2, 3, 4, 5, 6, 7]);
const state = new PostStreamState(discussion, postsFor(postIds.slice(0, 4)));

await state.syncEnd();

expect((state as any).visibleEnd).toBe(8);
expect(state.posts()).toHaveLength(8);
expect(state.posts().every((p) => p !== null)).toBe(true);
});
});
});
Loading