mirror of
https://github.com/supermemoryai/supermemory.git
synced 2026-10-11 03:37:56 +00:00
fix(tools): keep the forget-memory timeout when a caller passes a signal
`forgetMemoryRequest` combined the caller's signal and the 30s abort with `??`, making them mutually exclusive. Passing a cancellation signal removed the timeout, so a hung `DELETE /v4/memories` could wedge the tool call again — the exact condition #1451 set out to remove. There was also no way for a caller to ask for both cancellation and a timeout. Compose the two with `AbortSignal.any` instead of choosing between them. `AbortSignal.any` is available in Node 20.3+, Bun and workerd. No production call site passes `options` today (`ai-sdk.ts` and `openai/tools.ts` both omit it), so this was latent rather than live. The existing test asserted the buggy behaviour (`init.signal` being the caller's own signal), so it is replaced by two tests that pin the composed semantics: aborting the caller aborts the request, and the timeout leg still aborts the request on its own. Both fail against the previous implementation. Fixes #1549
This commit is contained in:
parent
0d90a15100
commit
444b4f1698
2 changed files with 39 additions and 3 deletions
|
|
@ -33,7 +33,11 @@ export async function forgetMemoryRequest(
|
|||
Authorization: `Bearer ${apiKey}`,
|
||||
},
|
||||
body: JSON.stringify(params),
|
||||
signal: options?.signal ?? AbortSignal.timeout(FETCH_TIMEOUT_MS),
|
||||
// Compose rather than choose: a caller-supplied signal must add cancellation
|
||||
// on top of the timeout, not replace it, or the request becomes unbounded.
|
||||
signal: options?.signal
|
||||
? AbortSignal.any([options.signal, AbortSignal.timeout(FETCH_TIMEOUT_MS)])
|
||||
: AbortSignal.timeout(FETCH_TIMEOUT_MS),
|
||||
})
|
||||
|
||||
if (!response.ok) {
|
||||
|
|
|
|||
|
|
@ -180,7 +180,7 @@ describe("memoryForget", () => {
|
|||
expect(init.signal).toBeInstanceOf(AbortSignal)
|
||||
})
|
||||
|
||||
it("uses a caller-provided signal instead of creating a timeout", async () => {
|
||||
it("cancels through a caller-provided signal", async () => {
|
||||
const fetchMock = stubFetch()
|
||||
const controller = new AbortController()
|
||||
|
||||
|
|
@ -192,7 +192,39 @@ describe("memoryForget", () => {
|
|||
)
|
||||
|
||||
const [, init] = fetchMock.mock.calls[0] as [string, RequestInit]
|
||||
expect(init.signal).toBe(controller.signal)
|
||||
// The request signal is a composite, not the caller's own, but aborting
|
||||
// the caller still aborts the request.
|
||||
expect(init.signal).not.toBe(controller.signal)
|
||||
controller.abort()
|
||||
expect(init.signal?.aborted).toBe(true)
|
||||
})
|
||||
|
||||
it("keeps the timeout when a caller-provided signal is present", async () => {
|
||||
const timeoutController = new AbortController()
|
||||
const timeoutSpy = vi
|
||||
.spyOn(AbortSignal, "timeout")
|
||||
.mockReturnValue(timeoutController.signal)
|
||||
const fetchMock = stubFetch()
|
||||
const controller = new AbortController()
|
||||
|
||||
try {
|
||||
await forgetMemoryRequest(
|
||||
API_KEY,
|
||||
{ containerTag: "user_1", id: "mem_1" },
|
||||
undefined,
|
||||
{ signal: controller.signal },
|
||||
)
|
||||
|
||||
expect(timeoutSpy).toHaveBeenCalledWith(30_000)
|
||||
|
||||
const [, init] = fetchMock.mock.calls[0] as [string, RequestInit]
|
||||
// Firing only the timeout leg aborts the request: a caller signal adds
|
||||
// cancellation, it does not remove the 30s bound.
|
||||
timeoutController.abort()
|
||||
expect(init.signal?.aborted).toBe(true)
|
||||
} finally {
|
||||
timeoutSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it("throws a descriptive error on non-2xx responses", async () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue