Skip to content

Commit 4c1b0a8

Browse files
committed
fix(suppress-warnings): don't clobber concurrent suppressions in finally
`withSuppressedWarnings` snapshotted `process.emitWarning` at entry and restored that snapshot in `finally`. If another caller had already installed the suppression wrapper (via `suppressMaxListenersWarning()` or a prior `suppressWarningType()`), the snapshot was the wrapper itself — restoring it was a harmless no-op. But if this was the first caller, the snapshot was the native `emitWarning` and the `finally` uninstalled the wrapper we just put in place. Any later `suppressWarningType()` calls would see `originalEmitWarning` already set and skip the re-wrap, so subsequent suppressions silently stopped working — a spooky-action regression that's hard to diagnose. The wrapper is data-driven by the `suppressedWarnings` set, so "restoring" simply means removing this type from the set. Drop the `process.emitWarning` reassignment entirely.
1 parent 5183fed commit 4c1b0a8

2 files changed

Lines changed: 22 additions & 9 deletions

File tree

src/suppress-warnings.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,15 +160,19 @@ export async function withSuppressedWarnings<T>(
160160
callback: () => T | Promise<T>,
161161
): Promise<T> {
162162
const wasAlreadySuppressed = suppressedWarnings.has(warningType)
163-
const original = process.emitWarning
164163
suppressWarningType(warningType)
165164
try {
166165
return await callback()
167166
} finally {
168-
// Only remove from suppressed set if we added it.
167+
// The wrapper is driven by `suppressedWarnings` membership, so
168+
// removing this type from the set is enough to stop suppressing it.
169+
// Do NOT reassign `process.emitWarning` here: snapshotting the
170+
// previous value at the top would either (a) capture the native
171+
// function when the wrapper was already installed and then restore
172+
// native — wiping every other active suppression — or (b) be the
173+
// wrapper itself, in which case the restore is a no-op anyway.
169174
if (!wasAlreadySuppressed) {
170175
suppressedWarnings.delete(warningType)
171176
}
172-
process.emitWarning = original
173177
}
174178
}

test/unit/suppress-warnings.test.mts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,24 @@ describe('suppress-warnings', () => {
161161
})
162162

163163
describe('withSuppressedWarnings', () => {
164-
it('should restore warnings after callback completes', async () => {
164+
it('does not wipe concurrent suppressions on exit', async () => {
165+
// Regression: the old implementation snapshotted
166+
// `process.emitWarning` at the top and restored it in `finally`.
167+
// If another caller had already installed a suppression wrapper,
168+
// `withSuppressedWarnings` would uninstall it on exit, silently
169+
// breaking the other caller's suppression. The fix is to not
170+
// reassign `process.emitWarning` in `finally` at all — the
171+
// wrapper is driven by the `suppressedWarnings` set.
165172
restoreWarnings()
166-
const original = process.emitWarning
167-
173+
// Outer caller: install a long-lived suppression wrapper.
174+
suppressWarningType('OtherWarning')
175+
const wrapperAfterOuter = process.emitWarning
176+
// Inner: withSuppressedWarnings runs and exits.
168177
await withSuppressedWarnings('TestWarning', () => {
169-
// Nothing
178+
// no-op
170179
})
171-
172-
expect(process.emitWarning).toBe(original)
180+
// Wrapper must still be installed so OtherWarning stays suppressed.
181+
expect(process.emitWarning).toBe(wrapperAfterOuter)
173182
})
174183

175184
it('should handle callback errors', async () => {

0 commit comments

Comments
 (0)