♻️ Extract util.callbackOrEmit() for synchronous error paths

[alecgibson]

At the moment, Doc branches on whether it was handed a callback in seven separate places, and Query and Backend do it once each:

if (callback) return callback(err);
return this.emit('error', err);

The idiom is already recognised elsewhere, but only as a private method copy-pasted onto three classes: Presence and LocalPresence carry byte-identical _callbackOrEmit() methods, and MilestoneDB has _callBackOrEmitError(), which differs in capitalisation and drops the falsy-error guard.

None of them was reusable here, because all three call back asynchronously via util.nextTick(), and every site in doc.js calls back synchronously. Adopting one verbatim would change when the error callbacks on doc.create(), doc.del() and doc.submitOp() fire, which is a breaking change for consumers rather than a refactor.

This change adds the synchronous helper to util, next to callEach(), and uses it at the nine sites whose behaviour it leaves untouched. It takes the emitter explicitly rather than relying on this, so it also works inside the nested callbacks in Doc#destroy() and Backend#close().

Three sites in doc.js deliberately keep their branch. _emitResponseError() interleaves _emitNothingPending() differently between its branches, and calls back with no error on a silent rejection. _hardRollback() and _clearInflightOp() use the array form — util.callEach() plus a called boolean — which is the shape of Presence._callEachOrEmit(), not this helper.

Note the shared helper is synchronous while the three private ones aren't, despite the near-identical name. The tests pin that down, but it's worth watching when the presence helpers are eventually folded in: mechanically rewriting this._callbackOrEmit(error, callback) to util.callbackOrEmit(this, error, callback) would quietly make every presence callback synchronous.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

[coveralls]

coverage: 97.783% (+0.3%) from 97.485% — extract-callback-or-emit-helper into main

[Copilot]

🟡 Changes recommended

util.callbackOrEmit() currently drops the emit() return value in the no-callback error path, subtly changing observable return semantics vs the previous inlined pattern.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR introduces a new synchronous util.callbackOrEmit() helper to centralize the common “callback if provided, otherwise emit error” branching, and then refactors Doc, Query, and Backend to use it in synchronous error paths without changing callback timing.

Changes:

• Added util.callbackOrEmit(emitter, error, callback) as a shared synchronous helper.
• Replaced repeated callback-vs-emit branching in lib/client/doc.js, lib/client/query.js, and lib/backend.js.
• Added unit tests covering the helper’s sync callback and emit behavior.

File summaries

File	Description
test/lib-util.js	Adds tests for util.callbackOrEmit() behavior (sync callback + emit/no-op cases).
lib/util.js	Adds the new callbackOrEmit helper near other util helpers.
lib/client/query.js	Refactors _finishResponse error path to use util.callbackOrEmit.
lib/client/doc.js	Refactors several synchronous error branches to use util.callbackOrEmit.
lib/backend.js	Refactors Backend#close() error handling to use util.callbackOrEmit.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.