diff --git a/.changeset/fluffy-spies-guess.md b/.changeset/fluffy-spies-guess.md new file mode 100644 index 000000000..3e802b932 --- /dev/null +++ b/.changeset/fluffy-spies-guess.md @@ -0,0 +1,5 @@ +--- +'@tanstack/pacer': patch +--- + +Guard retryer key propagation in AsyncQueuer, AsyncThrottler, AsyncRateLimiter, and AsyncDebouncer to prevent child AsyncRetryer instances from receiving a truthy key when the parent has no key, which caused unbounded devtools event accumulation and memory leaks in Node.js environments. diff --git a/packages/pacer/src/async-debouncer.ts b/packages/pacer/src/async-debouncer.ts index d24d721a7..481886343 100644 --- a/packages/pacer/src/async-debouncer.ts +++ b/packages/pacer/src/async-debouncer.ts @@ -368,7 +368,7 @@ export class AsyncDebouncer { this.#setState({ isExecuting: true }) const currentAsyncRetryer = new AsyncRetryer(this.fn, { ...this.options.asyncRetryerOptions, - key: `${this.key}-retryer-${currentMaybeExecuteCount}`, + key: this.key ? `${this.key}-retryer-${currentMaybeExecuteCount}` : undefined, }) this.asyncRetryers.set(currentMaybeExecuteCount, currentAsyncRetryer) const result = await currentAsyncRetryer.execute(...args) // EXECUTE! diff --git a/packages/pacer/src/async-queuer.ts b/packages/pacer/src/async-queuer.ts index 22925a170..d606cca87 100644 --- a/packages/pacer/src/async-queuer.ts +++ b/packages/pacer/src/async-queuer.ts @@ -618,7 +618,7 @@ export class AsyncQueuer { try { const currentAsyncRetryer = new AsyncRetryer(this.fn, { ...this.options.asyncRetryerOptions, - key: `${this.key}-retryer-${currentExecuteCount}`, + key: this.key ? `${this.key}-retryer-${currentExecuteCount}` : undefined, }) this.asyncRetryers.set(currentExecuteCount, currentAsyncRetryer) const lastResult = await currentAsyncRetryer.execute(item) // EXECUTE! diff --git a/packages/pacer/src/async-rate-limiter.ts b/packages/pacer/src/async-rate-limiter.ts index 5ac9be42d..58576fd11 100644 --- a/packages/pacer/src/async-rate-limiter.ts +++ b/packages/pacer/src/async-rate-limiter.ts @@ -395,7 +395,7 @@ export class AsyncRateLimiter { // Create a new AsyncRetryer for this execution to avoid cancelling concurrent executions const currentAsyncRetryer = new AsyncRetryer(this.fn, { ...this.options.asyncRetryerOptions, - key: `${this.key}-retryer-${currentMaybeExecute}`, + key: this.key ? `${this.key}-retryer-${currentMaybeExecute}` : undefined, }) this.asyncRetryers.set(currentMaybeExecute, currentAsyncRetryer) const result = await currentAsyncRetryer.execute(...args) // EXECUTE! diff --git a/packages/pacer/src/async-throttler.ts b/packages/pacer/src/async-throttler.ts index b6bc30b3a..a742a96a4 100644 --- a/packages/pacer/src/async-throttler.ts +++ b/packages/pacer/src/async-throttler.ts @@ -416,7 +416,7 @@ export class AsyncThrottler { this.#setState({ isExecuting: true }) const currentAsyncRetryer = new AsyncRetryer(this.fn, { ...this.options.asyncRetryerOptions, - key: `${this.key}-retryer-${currentMaybeExecute}`, + key: this.key ? `${this.key}-retryer-${currentMaybeExecute}` : undefined, }) this.asyncRetryers.set(currentMaybeExecute, currentAsyncRetryer) const result = await currentAsyncRetryer.execute(...args) // EXECUTE! diff --git a/packages/pacer/tests/async-debouncer.test.ts b/packages/pacer/tests/async-debouncer.test.ts index 66ac0ca99..eff0c7c44 100644 --- a/packages/pacer/tests/async-debouncer.test.ts +++ b/packages/pacer/tests/async-debouncer.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { AsyncDebouncer, asyncDebounce } from '../src/async-debouncer' +import { pacerEventClient } from '../src' describe('AsyncDebouncer', () => { beforeEach(() => { @@ -1359,4 +1360,88 @@ describe('asyncDebounce helper function', () => { expect(debouncer.getAbortSignal()).toBeNull() }) }) + + describe('retryer key propagation', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it('should give child AsyncRetryer key: undefined when debouncer has no key', async () => { + let resolve!: (v: string) => void + const gate = new Promise((r) => { resolve = r }) + + const debouncer = new AsyncDebouncer( + async () => { await gate }, + { wait: 100 }, + ) + + debouncer.maybeExecute() + await vi.advanceTimersByTimeAsync(100) + + const retryer = debouncer.asyncRetryers.get(2) + expect(retryer).toBeDefined() + expect(retryer!.key).toBeUndefined() + + resolve('done') + await vi.advanceTimersByTimeAsync(0) + }) + + it('should give child AsyncRetryer a namespaced key when debouncer has a key', async () => { + let resolve!: (v: string) => void + const gate = new Promise((r) => { resolve = r }) + + const debouncer = new AsyncDebouncer( + async () => { await gate }, + { wait: 100, key: 'my-debouncer' }, + ) + + debouncer.maybeExecute() + await vi.advanceTimersByTimeAsync(100) + + const retryer = debouncer.asyncRetryers.get(2) + expect(retryer).toBeDefined() + expect(retryer!.key).toBe('my-debouncer-retryer-2') + + resolve('done') + await vi.advanceTimersByTimeAsync(0) + }) + + it('should not queue devtools events when debouncer has no key', async () => { + const emitSpy = vi.spyOn(pacerEventClient, 'emit') + + const debouncer = new AsyncDebouncer( + async () => 'result', + { wait: 100 }, + ) + + emitSpy.mockClear() + + debouncer.maybeExecute() + await vi.advanceTimersByTimeAsync(100) + + const retryerEmits = emitSpy.mock.calls.filter( + ([event]) => event === 'AsyncRetryer', + ) + expect(retryerEmits).toHaveLength(0) + }) + + it('should queue devtools events when debouncer has a key', async () => { + const emitSpy = vi.spyOn(pacerEventClient, 'emit') + + const debouncer = new AsyncDebouncer( + async () => 'result', + { wait: 100, key: 'my-debouncer' }, + ) + + emitSpy.mockClear() + + debouncer.maybeExecute() + await vi.advanceTimersByTimeAsync(100) + + const retryerEmits = emitSpy.mock.calls.filter( + ([event]) => event === 'AsyncRetryer', + ) + expect(retryerEmits.length).toBeGreaterThan(0) + }) + }) }) diff --git a/packages/pacer/tests/async-queuer.test.ts b/packages/pacer/tests/async-queuer.test.ts index 5497797bc..c1e1bc369 100644 --- a/packages/pacer/tests/async-queuer.test.ts +++ b/packages/pacer/tests/async-queuer.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { AsyncQueuer } from '../src' +import { AsyncQueuer, pacerEventClient } from '../src' describe('AsyncQueuer', () => { beforeEach(() => { @@ -1095,4 +1095,90 @@ describe('AsyncQueuer', () => { expect(asyncQueuer.getAbortSignal()).toBeNull() }) }) + + describe('retryer key propagation', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it('should give child AsyncRetryer key: undefined when queuer has no key', async () => { + let resolve!: (v: string) => void + const item = new Promise((r) => { resolve = r }) + + const asyncQueuer = new AsyncQueuer>( + (p) => p, + { started: false }, + ) + + asyncQueuer.addItem(item) + const executePromise = asyncQueuer.execute() + + const retryer = asyncQueuer.asyncRetryers.get(1) + expect(retryer).toBeDefined() + expect(retryer!.key).toBeUndefined() + + resolve('done') + await executePromise + }) + + it('should give child AsyncRetryer a namespaced key when queuer has a key', async () => { + let resolve!: (v: string) => void + const item = new Promise((r) => { resolve = r }) + + const asyncQueuer = new AsyncQueuer>( + (p) => p, + { started: false, key: 'my-queue' }, + ) + + asyncQueuer.addItem(item) + const executePromise = asyncQueuer.execute() + + const retryer = asyncQueuer.asyncRetryers.get(1) + expect(retryer).toBeDefined() + expect(retryer!.key).toBe('my-queue-retryer-1') + + resolve('done') + await executePromise + }) + + it('should not queue devtools events when queuer has no key', async () => { + const emitSpy = vi.spyOn(pacerEventClient, 'emit') + + const asyncQueuer = new AsyncQueuer( + (item) => Promise.resolve(item), + { started: false }, + ) + + emitSpy.mockClear() + + asyncQueuer.addItem('a') + asyncQueuer.addItem('b') + await asyncQueuer.execute() + await asyncQueuer.execute() + + const retryerEmits = emitSpy.mock.calls.filter( + ([event]) => event === 'AsyncRetryer', + ) + expect(retryerEmits).toHaveLength(0) + }) + + it('should queue devtools events when queuer has a key', async () => { + const emitSpy = vi.spyOn(pacerEventClient, 'emit') + + const asyncQueuer = new AsyncQueuer( + (item) => Promise.resolve(item), + { started: false, key: 'my-queue' }, + ) + + emitSpy.mockClear() + + asyncQueuer.addItem('a') + await asyncQueuer.execute() + + const retryerEmits = emitSpy.mock.calls.filter( + ([event]) => event === 'AsyncRetryer', + ) + expect(retryerEmits.length).toBeGreaterThan(0) + }) + }) }) diff --git a/packages/pacer/tests/async-rate-limiter.test.ts b/packages/pacer/tests/async-rate-limiter.test.ts index 22bcae3c8..1e889faf0 100644 --- a/packages/pacer/tests/async-rate-limiter.test.ts +++ b/packages/pacer/tests/async-rate-limiter.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { AsyncRateLimiter, asyncRateLimit } from '../src/async-rate-limiter' +import { pacerEventClient } from '../src' describe('AsyncRateLimiter', () => { beforeEach(() => { @@ -775,4 +776,86 @@ describe('asyncRateLimit', () => { expect(rateLimiter.getAbortSignal()).toBeNull() }) }) + + describe('retryer key propagation', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it('should give child AsyncRetryer key: undefined when rate limiter has no key', async () => { + let resolve!: (v: string) => void + const gate = new Promise((r) => { resolve = r }) + + const rateLimiter = new AsyncRateLimiter( + async () => { await gate }, + { limit: 5, window: 1000 }, + ) + + const executePromise = rateLimiter.maybeExecute() + await vi.advanceTimersByTimeAsync(0) + + const retryer = rateLimiter.asyncRetryers.get(1) + expect(retryer).toBeDefined() + expect(retryer!.key).toBeUndefined() + + resolve('done') + await executePromise + }) + + it('should give child AsyncRetryer a namespaced key when rate limiter has a key', async () => { + let resolve!: (v: string) => void + const gate = new Promise((r) => { resolve = r }) + + const rateLimiter = new AsyncRateLimiter( + async () => { await gate }, + { limit: 5, window: 1000, key: 'my-limiter' }, + ) + + const executePromise = rateLimiter.maybeExecute() + await vi.advanceTimersByTimeAsync(0) + + const retryer = rateLimiter.asyncRetryers.get(1) + expect(retryer).toBeDefined() + expect(retryer!.key).toBe('my-limiter-retryer-1') + + resolve('done') + await executePromise + }) + + it('should not queue devtools events when rate limiter has no key', async () => { + const emitSpy = vi.spyOn(pacerEventClient, 'emit') + + const rateLimiter = new AsyncRateLimiter( + async () => 'result', + { limit: 5, window: 1000 }, + ) + + emitSpy.mockClear() + + await rateLimiter.maybeExecute() + + const retryerEmits = emitSpy.mock.calls.filter( + ([event]) => event === 'AsyncRetryer', + ) + expect(retryerEmits).toHaveLength(0) + }) + + it('should queue devtools events when rate limiter has a key', async () => { + const emitSpy = vi.spyOn(pacerEventClient, 'emit') + + const rateLimiter = new AsyncRateLimiter( + async () => 'result', + { limit: 5, window: 1000, key: 'my-limiter' }, + ) + + emitSpy.mockClear() + + await rateLimiter.maybeExecute() + + const retryerEmits = emitSpy.mock.calls.filter( + ([event]) => event === 'AsyncRetryer', + ) + expect(retryerEmits.length).toBeGreaterThan(0) + }) + }) }) diff --git a/packages/pacer/tests/async-throttler.test.ts b/packages/pacer/tests/async-throttler.test.ts index 2cb1072ef..fe487e4be 100644 --- a/packages/pacer/tests/async-throttler.test.ts +++ b/packages/pacer/tests/async-throttler.test.ts @@ -1,5 +1,6 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { AsyncThrottler } from '../src/async-throttler' +import { pacerEventClient } from '../src' describe('AsyncThrottler', () => { beforeEach(() => { @@ -998,4 +999,86 @@ describe('AsyncThrottler', () => { expect(throttler.getAbortSignal()).toBeNull() }) }) + + describe('retryer key propagation', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it('should give child AsyncRetryer key: undefined when throttler has no key', async () => { + let resolve!: (v: string) => void + const gate = new Promise((r) => { resolve = r }) + + const throttler = new AsyncThrottler( + async () => { await gate }, + { wait: 0 }, + ) + + const executePromise = throttler.maybeExecute() + await vi.advanceTimersByTimeAsync(0) + + const retryer = throttler.asyncRetryers.get(1) + expect(retryer).toBeDefined() + expect(retryer!.key).toBeUndefined() + + resolve('done') + await executePromise + }) + + it('should give child AsyncRetryer a namespaced key when throttler has a key', async () => { + let resolve!: (v: string) => void + const gate = new Promise((r) => { resolve = r }) + + const throttler = new AsyncThrottler( + async () => { await gate }, + { wait: 0, key: 'my-throttler' }, + ) + + const executePromise = throttler.maybeExecute() + await vi.advanceTimersByTimeAsync(0) + + const retryer = throttler.asyncRetryers.get(1) + expect(retryer).toBeDefined() + expect(retryer!.key).toBe('my-throttler-retryer-1') + + resolve('done') + await executePromise + }) + + it('should not queue devtools events when throttler has no key', async () => { + const emitSpy = vi.spyOn(pacerEventClient, 'emit') + + const throttler = new AsyncThrottler( + async () => 'result', + { wait: 0 }, + ) + + emitSpy.mockClear() + + await throttler.maybeExecute() + + const retryerEmits = emitSpy.mock.calls.filter( + ([event]) => event === 'AsyncRetryer', + ) + expect(retryerEmits).toHaveLength(0) + }) + + it('should queue devtools events when throttler has a key', async () => { + const emitSpy = vi.spyOn(pacerEventClient, 'emit') + + const throttler = new AsyncThrottler( + async () => 'result', + { wait: 0, key: 'my-throttler' }, + ) + + emitSpy.mockClear() + + await throttler.maybeExecute() + + const retryerEmits = emitSpy.mock.calls.filter( + ([event]) => event === 'AsyncRetryer', + ) + expect(retryerEmits.length).toBeGreaterThan(0) + }) + }) })