Skip to content

Commit c0a7e12

Browse files
committed
fix(api): prevent cleanup leaking into request config
1 parent cf41486 commit c0a7e12

2 files changed

Lines changed: 23 additions & 3 deletions

File tree

src/api/providers/__tests__/request-config-builder.spec.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ describe("RequestConfigBuilder", () => {
2424
const result = builder.build()
2525
expect(result?.modelId).toBe("test-model")
2626
})
27+
28+
test("should initialize cleanup as a no-op", () => {
29+
const builder = new RequestConfigBuilder()
30+
31+
expect(builder.getCleanup()).toBeTypeOf("function")
32+
expect(() => builder.getCleanup()()).not.toThrow()
33+
})
2734
})
2835

2936
describe("addAbortSignal", () => {
@@ -316,7 +323,8 @@ describe("RequestConfigBuilder", () => {
316323
expect(result).toBe(builder)
317324
const config = builder.build() as { signal?: AbortSignal; _cleanup?: () => void }
318325
expect(config.signal).toBe(internalController.signal)
319-
expect(config._cleanup).toBeTypeOf("function")
326+
expect(config).not.toHaveProperty("_cleanup")
327+
expect(builder.getCleanup()).toBeTypeOf("function")
320328
})
321329

322330
test("should merge internal controller signal with metadata abort signal", () => {
@@ -346,12 +354,13 @@ describe("RequestConfigBuilder", () => {
346354

347355
const config = builder.build() as { signal?: AbortSignal; _cleanup?: () => void }
348356
expect(config.signal).not.toBe(internalController.signal)
357+
expect(config).not.toHaveProperty("_cleanup")
349358
expect(config.signal?.aborted).toBe(false)
350359

351360
await vi.advanceTimersByTimeAsync(100)
352361

353362
expect(config.signal?.aborted).toBe(true)
354-
config._cleanup?.()
363+
builder.getCleanup()()
355364
vi.useRealTimers()
356365
})
357366
})

src/api/providers/config-builder/request-config-builder.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { mergeAbortSignalAndTimeout, mergeAbortSignals } from "../utils/abort-si
1313
*/
1414
export class RequestConfigBuilder<TOptions extends Record<string, any> = Record<string, any>> {
1515
protected options: TOptions
16+
private cleanupFn: () => void = () => {}
1617

1718
constructor(defaultOptions?: Partial<TOptions>) {
1819
this.options = (defaultOptions ? { ...defaultOptions } : {}) as TOptions
@@ -68,10 +69,20 @@ export class RequestConfigBuilder<TOptions extends Record<string, any> = Record<
6869
const merged = mergeAbortSignalAndTimeout(metadata?.abortSignal, timeoutMs)
6970
const signal = mergeAbortSignals(internalController.signal, merged.signal)
7071

71-
this.options = { ...this.options, signal, _cleanup: merged.cleanup } as TOptions
72+
this.options = { ...this.options, signal } as TOptions
73+
this.cleanupFn = merged.cleanup
7274
return this
7375
}
7476

77+
/**
78+
* Get the cleanup function for resources created by addMergedSignal.
79+
*
80+
* @returns Cleanup function, or a no-op when no merged signal was added
81+
*/
82+
getCleanup(): () => void {
83+
return this.cleanupFn
84+
}
85+
7586
/**
7687
* Set a single option by key (type-safe).
7788
*

0 commit comments

Comments
 (0)