Skip to content

fix: prevent DDPQueue unblock stack overflow via depth-based microtask breaking #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
welkinwong wants to merge 1 commit into from
Closed

fix: prevent DDPQueue unblock stack overflow via depth-based microtask breaking #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 16 additions & 1 deletion lib/collectors/DDPQueueCollector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export default class DDPQueueCollector {
constructor(options = {}) {
this.enabled = options.enabled !== false;
this.debug = options.debug || false;
this.unblockDepth = 0;

// Store wait lists for messages (who they're waiting for)
this.waitLists = {};
Expand Down Expand Up @@ -391,7 +392,21 @@ export default class DDPQueueCollector {
// itself be a wrapper (from MethodTracer or another layer), and retrying
// after a throw can cause infinite recursion across wrapper layers. See #7.
if (typeof originalUnblock === 'function') {
originalUnblock();
// Deep wrapper chains can still overflow the JS call stack even without
// retries. Once nesting gets too deep, defer one hop to break recursion.
if (self.unblockDepth >= 1000) {
queueMicrotask(() => {
originalUnblock();

Copilot AI Mar 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When originalUnblock() is deferred via queueMicrotask, any exception it throws will become an unhandled promise rejection (Node.js will emit an unhandledRejection or uncaughtException depending on the Node version), since there is no try/catch around the call inside the microtask. In the synchronous path, exceptions propagate to the caller. This inconsistency means that in the deferred path, errors from deep in the unblock chain will be silently lost or will crash the process, rather than being handled by the existing error-handling logic in the wrappers. A try/catch should be added inside the queueMicrotask callback to handle errors consistently with the synchronous path.

Suggested change
originalUnblock();
try {
originalUnblock();
} catch (error) {
// Ensure errors in the deferred path are handled and don't surface
// as unhandled rejections / uncaught exceptions.
console.error('⚠️ DDPQueueCollector: Error during deferred unblock:', error);
}

Copilot uses AI. Check for mistakes.
});
return;
}

self.unblockDepth++;
try {
originalUnblock();
} finally {
self.unblockDepth--;
}
}
};
}
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/collectors/DDPQueueCollector.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,28 @@ describe('DDPQueueCollector', function () {
// Should not throw
expect(() => wrappedUnblock()).to.not.throw();
});

it('breaks deep synchronous unblock chains to avoid stack overflow', function (done) {
const deepCollector = new DDPQueueCollector({ enabled: true });
const session = createMockSession('deep-session');
const meteorUnblock = sinon.stub();

let chained = meteorUnblock;
for (let i = 0; i < 5000; i++) {
chained = deepCollector.wrapUnblock(
session,
{ id: `deep-${i}`, msg: 'sub', name: 'assets.assetsInfo' },
chained
);
}

expect(() => chained()).to.not.throw();

setTimeout(() => {
expect(meteorUnblock.calledOnce).to.be.true;
done();
}, 20);
});
});

describe('_wrapSession', function () {
Expand Down