diff --git a/.changeset/gentle-hornets-relax.md b/.changeset/gentle-hornets-relax.md new file mode 100644 index 00000000000..10125bcade3 --- /dev/null +++ b/.changeset/gentle-hornets-relax.md @@ -0,0 +1,5 @@ +--- +"@smithy/util-waiter": patch +--- + +clean up waiters' abort signal listener diff --git a/packages/util-waiter/src/createWaiter.spec.ts b/packages/util-waiter/src/createWaiter.spec.ts index 4089fb22310..5391205f61d 100644 --- a/packages/util-waiter/src/createWaiter.spec.ts +++ b/packages/util-waiter/src/createWaiter.spec.ts @@ -1,14 +1,12 @@ -import { AbortController } from "@smithy/abort-controller"; import { afterEach, beforeEach, describe, expect, test as it, vi } from "vitest"; +import { createWaiter } from "./createWaiter"; import { WaiterOptions, WaiterState } from "./waiter"; vi.mock("./utils/validate", () => ({ validateWaiterOptions: vi.fn(), })); -import { createWaiter } from "./createWaiter"; - describe("createWaiter", () => { beforeEach(() => { vi.useFakeTimers(); @@ -56,7 +54,28 @@ describe("createWaiter", () => { expect(await statusPromise).toMatchObject(abortedState); }); - it("should success when acceptor checker returns seccess", async () => { + it("should remove the event listener on the abort signal after the waiter resolves regardless of whether it has been invoked", async () => { + const abortController = new AbortController(); + vi.spyOn(abortController.signal, "addEventListener"); + vi.spyOn(abortController.signal, "removeEventListener"); + + const mockAcceptorChecks = vi.fn().mockResolvedValue(successState); + const statusPromise = createWaiter( + { + ...minimalWaiterConfig, + abortSignal: abortController.signal, + maxWaitTime: 20, + }, + input, + mockAcceptorChecks + ); + expect(abortController.signal.addEventListener).toHaveBeenCalledOnce(); + vi.advanceTimersByTime(minimalWaiterConfig.minDelay * 1000); + expect(await statusPromise).toMatchObject(successState); + expect(abortController.signal.removeEventListener).toHaveBeenCalledOnce(); + }); + + it("should succeed when acceptor checker returns success", async () => { const mockAcceptorChecks = vi.fn().mockResolvedValue(successState); const statusPromise = createWaiter( { diff --git a/packages/util-waiter/src/createWaiter.ts b/packages/util-waiter/src/createWaiter.ts index 52df3ccdb30..5b3ae20f2fb 100644 --- a/packages/util-waiter/src/createWaiter.ts +++ b/packages/util-waiter/src/createWaiter.ts @@ -4,9 +4,16 @@ import { runPolling } from "./poller"; import { validateWaiterOptions } from "./utils"; import { WaiterOptions, WaiterResult, waiterServiceDefaults, WaiterState } from "./waiter"; -const abortTimeout = async (abortSignal: AbortSignal | DeprecatedAbortSignal): Promise => { - return new Promise((resolve) => { - const onAbort = () => resolve({ state: WaiterState.ABORTED }); +const abortTimeout = ( + abortSignal: AbortSignal | DeprecatedAbortSignal +): { + clearListener: () => void; + aborted: Promise; +} => { + let onAbort: () => void; + + const promise = new Promise((resolve) => { + onAbort = () => resolve({ state: WaiterState.ABORTED }); if (typeof (abortSignal as AbortSignal).addEventListener === "function") { // preferred. (abortSignal as AbortSignal).addEventListener("abort", onAbort); @@ -15,6 +22,15 @@ const abortTimeout = async (abortSignal: AbortSignal | DeprecatedAbortSignal): P abortSignal.onabort = onAbort; } }); + + return { + clearListener() { + if (typeof (abortSignal as AbortSignal).removeEventListener === "function") { + (abortSignal as AbortSignal).removeEventListener("abort", onAbort); + } + }, + aborted: promise, + }; }; /** @@ -38,13 +54,24 @@ export const createWaiter = async ( validateWaiterOptions(params); const exitConditions = [runPolling(params, input, acceptorChecks)]; - if (options.abortController) { - exitConditions.push(abortTimeout(options.abortController.signal)); - } + + const finalize = [] as Array<() => void>; if (options.abortSignal) { - exitConditions.push(abortTimeout(options.abortSignal)); + const { aborted, clearListener } = abortTimeout(options.abortSignal); + finalize.push(clearListener); + exitConditions.push(aborted); + } + if (options.abortController?.signal) { + const { aborted, clearListener } = abortTimeout(options.abortController.signal); + finalize.push(clearListener); + exitConditions.push(aborted); } - return Promise.race(exitConditions); + return Promise.race(exitConditions).then((result) => { + for (const fn of finalize) { + fn(); + } + return result; + }); };