Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: switch to platform AbortController & Signal implementations after dropping Node.js 14 #1308

Merged
merged 10 commits into from
Jun 18, 2024
9 changes: 9 additions & 0 deletions .changeset/spotty-cars-do.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@smithy/fetch-http-handler": minor
"@smithy/node-http-handler": minor
"@smithy/abort-controller": minor
"@smithy/util-waiter": minor
"@smithy/types": minor
---

use platform AbortController|AbortSignal implementations
11 changes: 7 additions & 4 deletions packages/abort-controller/src/AbortController.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
import { AbortController as IAbortController } from "@smithy/types";
import { AbortController as DeprecatedAbortController } from "@smithy/types";

import { AbortSignal } from "./AbortSignal";

export { IAbortController };
/**
* @public
*/
export { DeprecatedAbortController as IAbortController };

/**
* This implementation was added as Node.js didn't support AbortController prior to 15.x
* @deprecated This implementation was added as Node.js didn't support AbortController prior to 15.x
* Use native implementation in browsers or Node.js \>=15.4.0.
*
* @public
*/
export class AbortController implements IAbortController {
export class AbortController implements DeprecatedAbortController {
public readonly signal: AbortSignal = new AbortSignal();

abort(): void {
Expand Down
9 changes: 6 additions & 3 deletions packages/abort-controller/src/AbortSignal.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { AbortHandler, AbortSignal as IAbortSignal } from "@smithy/types";
import { AbortHandler, AbortSignal as DeprecatedAbortSignal } from "@smithy/types";

export { AbortHandler, IAbortSignal };
/**
* @public
*/
export { AbortHandler, DeprecatedAbortSignal as IAbortSignal };

/**
* @public
*/
export class AbortSignal implements IAbortSignal {
export class AbortSignal implements DeprecatedAbortSignal {
public onabort: AbortHandler | null = null;
private _aborted = false;

Expand Down
9 changes: 8 additions & 1 deletion packages/fetch-http-handler/src/fetch-http-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,18 @@ export class FetchHttpHandler implements HttpHandler<FetchHttpHandlerConfig> {
if (abortSignal) {
raceOfPromises.push(
new Promise<never>((resolve, reject) => {
abortSignal.onabort = () => {
const onAbort = () => {
const abortError = new Error("Request aborted");
abortError.name = "AbortError";
reject(abortError);
};
if (typeof (abortSignal as AbortSignal).addEventListener === "function") {
// preferred.
(abortSignal as AbortSignal).addEventListener("abort", onAbort);
} else {
// backwards compatibility
abortSignal.onabort = onAbort;
}
})
);
}
Expand Down
6 changes: 3 additions & 3 deletions packages/node-http-handler/src/node-http-handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -619,12 +619,12 @@ describe("NodeHttpHandler", () => {
};
mockHttpsServer.addListener("request", createResponseFunction(mockResponse));
let httpRequest: http.ClientRequest;
let reqAbortSpy: any;
let reqDestroySpy: any;
const spy = jest.spyOn(https, "request").mockImplementationOnce(() => {
const calls = spy.mock.calls;
const currentIndex = calls.length - 1;
httpRequest = https.request(calls[currentIndex][0], calls[currentIndex][1]);
reqAbortSpy = jest.spyOn(httpRequest, "abort");
reqDestroySpy = jest.spyOn(httpRequest, "destroy");
return httpRequest;
});
const nodeHttpHandler = new NodeHttpHandler();
Expand All @@ -650,7 +650,7 @@ describe("NodeHttpHandler", () => {
)
).rejects.toHaveProperty("name", "AbortError");

expect(reqAbortSpy.mock.calls.length).toBe(1);
expect(reqDestroySpy.mock.calls.length).toBe(1);
});
});

Expand Down
11 changes: 9 additions & 2 deletions packages/node-http-handler/src/node-http-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,20 @@ export class NodeHttpHandler implements HttpHandler<NodeHttpHandlerOptions> {

// wire-up abort logic
if (abortSignal) {
abortSignal.onabort = () => {
const onAbort = () => {
// ensure request is destroyed
req.abort();
req.destroy();
const abortError = new Error("Request aborted");
abortError.name = "AbortError";
reject(abortError);
};
if (typeof (abortSignal as AbortSignal).addEventListener === "function") {
// preferred.
(abortSignal as AbortSignal).addEventListener("abort", onAbort);
} else {
// backwards compatibility
abortSignal.onabort = onAbort;
}
}

// Workaround for bug report in Node.js https://github.com/nodejs/node/issues/47137
Expand Down
9 changes: 8 additions & 1 deletion packages/node-http-handler/src/node-http2-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,19 @@ export class NodeHttp2Handler implements HttpHandler<NodeHttp2HandlerOptions> {
}

if (abortSignal) {
abortSignal.onabort = () => {
const onAbort = () => {
req.close();
const abortError = new Error("Request aborted");
abortError.name = "AbortError";
rejectWithDestroy(abortError);
};
if (typeof (abortSignal as AbortSignal).addEventListener === "function") {
// preferred.
(abortSignal as AbortSignal).addEventListener("abort", onAbort);
} else {
// backwards compatibility
abortSignal.onabort = onAbort;
}
}

// Set up handlers for errors
Expand Down
8 changes: 8 additions & 0 deletions packages/types/src/abort-handler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import type { AbortSignal as DeprecatedAbortSignal } from "./abort";

/**
* @public
*/
export interface AbortHandler {
(this: AbortSignal | DeprecatedAbortSignal, ev: any): any;
}
Comment on lines +6 to +8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit question: will this ever need to be scoped down or have extra arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be ev: Event -> void, but I wouldn't change it at this point for compatibility

8 changes: 5 additions & 3 deletions packages/types/src/abort.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import type { AbortHandler } from "./abort-handler";

/**
* @public
*/
export interface AbortHandler {
(this: AbortSignal, ev: any): any;
}
export { AbortHandler };

/**
* @public
* @deprecated use platform (global) type for AbortSignal.
*
* Holders of an AbortSignal object may query if the associated operation has
* been aborted and register an onabort handler.
Expand All @@ -28,6 +29,7 @@ export interface AbortSignal {

/**
* @public
* @deprecated use platform (global) type for AbortController.
*
* The AWS SDK uses a Controller/Signal model to allow for cooperative
* cancellation of asynchronous operations. When initiating such an operation,
Expand Down
4 changes: 2 additions & 2 deletions packages/types/src/http.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AbortSignal } from "./abort";
import { AbortSignal as DeprecatedAbortSignal } from "./abort";
import { URI } from "./uri";

/**
Expand Down Expand Up @@ -106,7 +106,7 @@ export interface HttpMessage {
* Represents the options that may be passed to an Http Handler.
*/
export interface HttpHandlerOptions {
abortSignal?: AbortSignal;
abortSignal?: AbortSignal | DeprecatedAbortSignal;

/**
* The maximum time in milliseconds that the connection phase of a request
Expand Down
6 changes: 3 additions & 3 deletions packages/types/src/waiter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AbortController } from "./abort";
import { AbortController as DeprecatedAbortController } from "./abort";

/**
* @public
Expand All @@ -18,12 +18,12 @@ export interface WaiterConfiguration<Client> {
* @deprecated Use abortSignal
* Abort controller. Used for ending the waiter early.
*/
abortController?: AbortController;
abortController?: AbortController | DeprecatedAbortController;

/**
* Abort Signal. Used for ending the waiter early.
*/
abortSignal?: AbortController["signal"];
abortSignal?: AbortController["signal"] | DeprecatedAbortController["signal"];

/**
* The minimum amount of time to delay between retries in seconds. This is the
Expand Down
13 changes: 10 additions & 3 deletions packages/util-waiter/src/createWaiter.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import { AbortSignal } from "@smithy/types";
import { AbortSignal as DeprecatedAbortSignal } from "@smithy/types";

import { runPolling } from "./poller";
import { validateWaiterOptions } from "./utils";
import { WaiterOptions, WaiterResult, waiterServiceDefaults, WaiterState } from "./waiter";

const abortTimeout = async (abortSignal: AbortSignal): Promise<WaiterResult> => {
const abortTimeout = async (abortSignal: AbortSignal | DeprecatedAbortSignal): Promise<WaiterResult> => {
return new Promise((resolve) => {
abortSignal.onabort = () => resolve({ state: WaiterState.ABORTED });
const onAbort = () => resolve({ state: WaiterState.ABORTED });
if (typeof (abortSignal as AbortSignal).addEventListener === "function") {
// preferred.
(abortSignal as AbortSignal).addEventListener("abort", onAbort);
} else {
// backwards compatibility
abortSignal.onabort = onAbort;
}
});
};

Expand Down
Loading