mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-17 16:47:05 +00:00
fix(engine-flush-defer): address Qodo review
Two issues from the initial review: 1. Install guard blocked retries. Setting `installed = true` BEFORE requiring + validating engine.io meant a transient require error permanently disabled the patch for the rest of the process. Now the flag is set only after both checks pass; on failure, the warning is logged and a later boot path can retry. 2. engineFlushDefer was undocumented in settings.json.template. Added with the same prose as Settings.ts, including the "wire bytes are unchanged" / "no meaningful wall-clock latency" notes so operators see the safety case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
073954d62e
commit
f0661a9d8e
2 changed files with 20 additions and 3 deletions
|
|
@ -733,6 +733,19 @@
|
|||
*/
|
||||
"loadTest": false,
|
||||
|
||||
/*
|
||||
* Defer engine.io socket flush onto the next microtask so multiple
|
||||
* sendPacket() calls in the same task accumulate in writeBuffer before
|
||||
* the underlying transport.send drains. Pairs with engine.io's existing
|
||||
* batched-send path so high-fan-out scenarios produce fewer WebSocket
|
||||
* frames. Microtask deferral adds no meaningful wall-clock latency —
|
||||
* microtasks drain before any subsequent macrotask. Wire bytes are
|
||||
* unchanged.
|
||||
*
|
||||
* #7756 / #7767. Default off; production unaffected.
|
||||
*/
|
||||
"engineFlushDefer": false,
|
||||
|
||||
/**
|
||||
* Disable dump of objects preventing a clean exit
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -47,7 +47,6 @@ const SCHEDULED = Symbol('engineFlushScheduled');
|
|||
|
||||
export const installEngineFlushDeferral = (): void => {
|
||||
if (installed) return;
|
||||
installed = true;
|
||||
|
||||
let SocketProto: {sendPacket: (...a: unknown[]) => unknown};
|
||||
try {
|
||||
|
|
@ -55,12 +54,17 @@ export const installEngineFlushDeferral = (): void => {
|
|||
SocketProto = require('engine.io/build/socket').Socket.prototype;
|
||||
} catch (err: any) {
|
||||
logger.warn(`Unable to install engine.io flush deferral (module not found): ${err && err.message || err}`);
|
||||
return;
|
||||
return; // Leave `installed` false so a later boot path can retry.
|
||||
}
|
||||
if (typeof SocketProto.sendPacket !== 'function') {
|
||||
logger.warn('engine.io Socket shape unexpected; skipping flush deferral patch');
|
||||
return;
|
||||
return; // Leave `installed` false so a later boot path can retry.
|
||||
}
|
||||
// Only after both require and shape check succeed do we record that the
|
||||
// patch is installed. Setting the flag before validation (the original
|
||||
// code) would have permanently disabled retries after a transient
|
||||
// require failure in test/CI environments where socket.io may load late.
|
||||
installed = true;
|
||||
|
||||
// Re-implementing sendPacket inline rather than wrapping the original
|
||||
// so the single closing `this.flush()` becomes a microtask-coalesced
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue