Better background-ping support

[anoek]

Background tabs have different execution behaviors, as in heavily throttled timers and such. Because of that, when we're in the background, we're not going to update our latency or aggressively timeout the socket, when the tab wakes up then we'll restore our more proactive connection maintenance.

[github-actions]

Bug: clearTimeout fires before the stale-pong guard

In the new net/pong handler, clearTimeout(this.timeout_timer) and this.timeout_timer = undefined are placed unconditionally before the early-return filter:

this.on("net/pong", ({ client, server }) => {
    if (this.timeout_timer) {
        clearTimeout(this.timeout_timer);   // <-- runs for ALL pongs
        this.timeout_timer = undefined;
    }

    if (this.options.background_pinging ||
        (this.options.ignore_pongs_before && client < this.options.ignore_pongs_before)) {
        return;   // stale pong discarded, but timer already cleared
    }
    // ...
});

Problematic sequence:

1. Foreground ping sent at T1, timeout_timer armed.
2. Tab goes to background (background_pinging = true, ignore_pongs_before = T2).
3. Tab returns to foreground (background_pinging = false, ignore_pongs_before = T3).
4. New foreground ping sent at T4 > T3, new timeout_timer armed.
5. A stale pong for T1 arrives late → clearTimeout disarms the T4 timer unconditionally.
6. The stale-pong guard (T1 < T3) fires → early return.

The live foreground timeout_timer is now silently gone. If the connection dies before the next ping cycle re-arms it, no "timeout" event is emitted.

Fix: move the clearTimeout block to after the early-return guard, so it only runs for pongs that are actually accepted:

this.on("net/pong", ({ client, server }) => {
    if (this.options.background_pinging ||
        (this.options.ignore_pongs_before && client < this.options.ignore_pongs_before)) {
        return;
    }

    if (this.timeout_timer) {
        clearTimeout(this.timeout_timer);
        this.timeout_timer = undefined;
    }

    const now = Date.now();
    // ...
});

[github-actions]

Review Notes

Bug: Stale background pong cancels the active foreground timeout timer

In the net/pong early-return path, timeout_timer is unconditionally cleared whenever a pong is discarded — whether because background_pinging is still true, or because client < ignore_pongs_before:

if (
    this.options.background_pinging ||
    (this.options.ignore_pongs_before && client < this.options.ignore_pongs_before)
) {
    if (this.timeout_timer) {
        clearTimeout(this.timeout_timer);
        this.timeout_timer = undefined;
    }
    return;
}

Consider the background → foreground transition sequence:

1. Foreground ping fires → timeout_timer = T_new is armed.
2. A stale background-era pong arrives (its client timestamp predates ignore_pongs_before).
3. The guard matches the second condition and clears T_new, leaving no active timeout watchdog.

The connection can now go dead immediately after returning to the foreground and no timeout event will fire — exactly the scenario where timeout detection matters most.

The issue is that timeout_timer is shared with no way to tell which ping generation armed it. One fix is to tag the timer with the ping's client timestamp and only cancel it in the discard path if the tags match, so that the freshly-armed foreground timer is not collateral damage from a stale pong.

---

Design concern: ignore_pongs_before is never reset by the class

The field is set externally by callers and never cleared internally. On a second background/foreground cycle, if the caller forgets to update ignore_pongs_before to the new transition timestamp, pongs from the second background period (whose client timestamps are after the old value) will pass through the guard and pollute latency measurements. Since the API contract here is implicit, this is easy to get wrong silently.

[anoek]

@claude pretty sure it's safe to clear that timer right there and then attempt to clear it again after all the rest of the code potentially runs. Also I don't think we want to manage that ignore_pongs_before field, there's no reason to and if there were somehow multiple pongs that came back that were before that field we'd want to ignore them all.

[claude]

Claude finished @anoek's task in 50s —— View job

---

Analysis complete

• Read the actual code to understand the control flow
• Analyze whether clearing the timer before the early-return is safe
• Respond with assessment

---

You're correct on both points. Looking at the actual code, the automated reviews were describing a slightly different structure than what's actually there — the clearTimeout is already inside the discard branch, not unconditionally before it. And the reasoning holds.

On clearing the timer in the discard path:

The scenario the reviews flagged (stale pong clears the freshly-armed foreground timer) can happen, but it's not a silent permanent failure:

• A stale pong arriving at all means the server is alive and responding — which is exactly what the timeout is meant to detect
• ping() only arms timeout_timer when !this.options.background_pinging (line 239), so the timer will be re-armed on the next ping cycle
• The worst case is a one-ping_interval window (default 10s) with no timer armed — not a permanent watchdog hole

Clearing in both the discard and accept paths is safe. clearTimeout on an already-cleared timer is a no-op anyway.

On ignore_pongs_before management:

Agreed — the class shouldn't own that lifecycle. It's a caller-set filter for a specific transition moment, and if multiple stale pongs trickle in after a background→foreground switch, rejecting them all is the correct behavior. Managing it internally would just add complexity for no benefit.

The automated review comments appear to be false positives here.