feat(http-handlers): add support for per-request/per-operation timeouts (smithy-lang#1636)

* feat(handlers): add support for per-request timeouts

* chore: changeset

* chore: formatting

* fix(node-http-handler): explicitly type options obj

Author: siddsriv

.changeset/gentle-balloons-drum.md

@@ -0,0 +1,6 @@
+---
+"@smithy/fetch-http-handler": minor
+"@smithy/node-http-handler": minor
+---
+
+per-request timeouts support

packages/fetch-http-handler/src/fetch-http-handler.spec.ts

@@ -301,6 +301,37 @@ const globalFetch = global.fetch;
     expect(mockFetch.mock.calls.length).toBe(1);
   });
 
+  describe("per-request requestTimeout", () => {
+    it("should use per-request timeout over handler config timeout", async () => {
+      const mockFetch = vi.fn(() => new Promise(() => {})); // never resolve
+      (global as any).fetch = mockFetch;
+
+      const fetchHttpHandler = new FetchHttpHandler({ requestTimeout: 5000 });
+
+      const start = Date.now();
+      await expect(fetchHttpHandler.handle({} as any, { requestTimeout: 50 })).rejects.toHaveProperty(
+        "name",
+        "TimeoutError"
+      );
+
+      const elapsed = Date.now() - start;
+      expect(elapsed).toBeLessThan(100); // should timeout quickly
+    });
+
+    it("should fall back to handler config timeout when per-request timeout not provided", async () => {
+      const mockFetch = vi.fn(() => new Promise(() => {}));
+      (global as any).fetch = mockFetch;
+
+      const fetchHttpHandler = new FetchHttpHandler({ requestTimeout: 50 });
+
+      const start = Date.now();
+      await expect(fetchHttpHandler.handle({} as any, {})).rejects.toHaveProperty("name", "TimeoutError");
+
+      const elapsed = Date.now() - start;
+      expect(elapsed).toBeLessThan(100);
+    });
+  });
+
   it("will throw timeout error it timeout finishes before request", async () => {
     const mockFetch = vi.fn(() => {
       return new Promise(() => {});

packages/fetch-http-handler/src/fetch-http-handler.ts

@@ -4,7 +4,7 @@ import type { FetchHttpHandlerOptions } from "@smithy/types";
 import { HeaderBag, HttpHandlerOptions, Provider } from "@smithy/types";
 
 import { createRequest } from "./create-request";
-import { requestTimeout } from "./request-timeout";
+import { requestTimeout as requestTimeoutFn } from "./request-timeout";
 
 declare let AbortController: any;
 
@@ -73,11 +73,14 @@ export class FetchHttpHandler implements HttpHandler<FetchHttpHandlerOptions> {
     // Do nothing. TLS and HTTP/2 connection pooling is handled by the browser.
   }
 
-  async handle(request: HttpRequest, { abortSignal }: HttpHandlerOptions = {}): Promise<{ response: HttpResponse }> {
+  async handle(
+    request: HttpRequest,
+    { abortSignal, requestTimeout }: HttpHandlerOptions = {}
+  ): Promise<{ response: HttpResponse }> {
     if (!this.config) {
       this.config = await this.configProvider;
     }
-    const requestTimeoutInMs = this.config!.requestTimeout;
+    const requestTimeoutInMs = requestTimeout ?? this.config!.requestTimeout;
     const keepAlive = this.config!.keepAlive === true;
     const credentials = this.config!.credentials as RequestInit["credentials"];
 
@@ -175,7 +178,7 @@ export class FetchHttpHandler implements HttpHandler<FetchHttpHandlerOptions> {
           }),
         };
       }),
-      requestTimeout(requestTimeoutInMs),
+      requestTimeoutFn(requestTimeoutInMs),
     ];
     if (abortSignal) {
       raceOfPromises.push(

packages/node-http-handler/src/node-http-handler.spec.ts

@@ -258,4 +258,51 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf
       );
     });
   });
+
+  describe("per-request requestTimeout", () => {
+    it("should use per-request timeout over handler config timeout", () => {
+      const nodeHttpHandler = new NodeHttpHandler({ requestTimeout: 5000 });
+      const mockHandle = vi.spyOn(nodeHttpHandler, "handle");
+      const testTimeout = (handlerTimeout: number, requestTimeout?: number) => {
+        const handler = new NodeHttpHandler({ requestTimeout: handlerTimeout });
+        const options = requestTimeout !== undefined ? { requestTimeout } : {};
+        const expectedTimeout = requestTimeout ?? handlerTimeout;
+        return expectedTimeout;
+      };
+
+      // per-request timeout takes precedence
+      expect(testTimeout(5000, 100)).toBe(100);
+
+      // fallback to handler config
+      expect(testTimeout(200, undefined)).toBe(200);
+      expect(testTimeout(200)).toBe(200);
+    });
+
+    it("should pass correct timeout values to internal functions", async () => {
+      const nodeHttpHandler = new NodeHttpHandler({ requestTimeout: 5000 });
+      (nodeHttpHandler as any).config = {
+        requestTimeout: 5000,
+        httpAgent: new http.Agent(),
+        httpsAgent: new https.Agent(),
+        logger: console,
+      };
+
+      const httpRequest = new HttpRequest({
+        hostname: "example.com",
+        method: "GET",
+        protocol: "http:",
+        path: "/",
+        headers: {},
+      });
+
+      const options1 = { requestTimeout: 100 };
+      const options2: { requestTimeout?: number } = {};
+
+      const effectiveTimeout1 = options1.requestTimeout ?? (nodeHttpHandler as any).config.requestTimeout;
+      const effectiveTimeout2 = options2.requestTimeout ?? (nodeHttpHandler as any).config.requestTimeout;
+
+      expect(effectiveTimeout1).toBe(100); // per-request timeout
+      expect(effectiveTimeout2).toBe(5000); // handler config timeout
+    });
+  });
 });

packages/node-http-handler/src/node-http-handler.ts

@@ -152,7 +152,10 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf
     this.config?.httpsAgent?.destroy();
   }
 
-  async handle(request: HttpRequest, { abortSignal }: HttpHandlerOptions = {}): Promise<{ response: HttpResponse }> {
+  async handle(
+    request: HttpRequest,
+    { abortSignal, requestTimeout }: HttpHandlerOptions = {}
+  ): Promise<{ response: HttpResponse }> {
     if (!this.config) {
       this.config = await this.configProvider;
     }
@@ -281,8 +284,9 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf
 
       // Defer registration of socket event listeners if the connection and request timeouts
       // are longer than a few seconds. This avoids slowing down faster operations.
+      const effectiveRequestTimeout = requestTimeout ?? this.config.requestTimeout;
       timeouts.push(setConnectionTimeout(req, reject, this.config.connectionTimeout));
-      timeouts.push(setSocketTimeout(req, reject, this.config.requestTimeout));
+      timeouts.push(setSocketTimeout(req, reject, effectiveRequestTimeout));
 
       // Workaround for bug report in Node.js https://github.com/nodejs/node/issues/47137
       const httpAgent = nodeHttpsOptions.agent;
@@ -297,7 +301,7 @@ or increase socketAcquisitionWarningTimeout=(millis) in the NodeHttpHandler conf
         );
       }
 
-      writeRequestBodyPromise = writeRequestBody(req, request, this.config.requestTimeout).catch((e) => {
+      writeRequestBodyPromise = writeRequestBody(req, request, effectiveRequestTimeout).catch((e) => {
         timeouts.forEach(timing.clearTimeout);
         return _reject(e);
       });

packages/node-http-handler/src/node-http2-handler.spec.ts

@@ -565,6 +565,45 @@ describe(NodeHttp2Handler.name, () => {
     );
   });
 
+  describe("per-request requestTimeout", () => {
+    it("should use per-request timeout over handler config timeout", async () => {
+      const nodeH2Handler = new NodeHttp2Handler({ requestTimeout: 5000 });
+
+      const mockH2Server = mockH2Servers[port1];
+      mockH2Server.removeAllListeners("request");
+      mockH2Server.on("request", () => {
+        // don't respond - let it timeout
+      });
+
+      const mockRequest = new HttpRequest(getMockReqOptions());
+
+      const start = Date.now();
+      await expect(nodeH2Handler.handle(mockRequest, { requestTimeout: 100 })).rejects.toHaveProperty(
+        "name",
+        "TimeoutError"
+      );
+
+      const elapsed = Date.now() - start;
+      expect(elapsed).toBeLessThan(200);
+    });
+
+    it("should fall back to handler config timeout when per-request timeout not provided", async () => {
+      const nodeH2Handler = new NodeHttp2Handler({ requestTimeout: 100 });
+
+      const mockH2Server = mockH2Servers[port1];
+      mockH2Server.removeAllListeners("request");
+      mockH2Server.on("request", () => {});
+
+      const mockRequest = new HttpRequest(getMockReqOptions());
+
+      const start = Date.now();
+      await expect(nodeH2Handler.handle(mockRequest, {})).rejects.toHaveProperty("name", "TimeoutError");
+
+      const elapsed = Date.now() - start;
+      expect(elapsed).toBeLessThan(200);
+    });
+  });
+
   describe.each([
     ["object provider", async () => ({ disableConcurrentStreams: true })],
     ["static object", { disableConcurrentStreams: true }],

packages/node-http-handler/src/node-http2-handler.ts

@@ -87,15 +87,19 @@ export class NodeHttp2Handler implements HttpHandler<NodeHttp2HandlerOptions> {
     this.connectionManager.destroy();
   }
 
-  async handle(request: HttpRequest, { abortSignal }: HttpHandlerOptions = {}): Promise<{ response: HttpResponse }> {
+  async handle(
+    request: HttpRequest,
+    { abortSignal, requestTimeout }: HttpHandlerOptions = {}
+  ): Promise<{ response: HttpResponse }> {
     if (!this.config) {
       this.config = await this.configProvider;
       this.connectionManager.setDisableConcurrentStreams(this.config.disableConcurrentStreams || false);
       if (this.config.maxConcurrentStreams) {
         this.connectionManager.setMaxConcurrentStreams(this.config.maxConcurrentStreams);
       }
     }
-    const { requestTimeout, disableConcurrentStreams } = this.config;
+    const { requestTimeout: configRequestTimeout, disableConcurrentStreams } = this.config;
+    const effectiveRequestTimeout = requestTimeout ?? configRequestTimeout;
     return new Promise((_resolve, _reject) => {
       // It's redundant to track fulfilled because promises use the first resolution/rejection
       // but avoids generating unnecessary stack traces in the "close" event handler.
@@ -176,10 +180,10 @@ export class NodeHttp2Handler implements HttpHandler<NodeHttp2HandlerOptions> {
         }
       });
 
-      if (requestTimeout) {
-        req.setTimeout(requestTimeout, () => {
+      if (effectiveRequestTimeout) {
+        req.setTimeout(effectiveRequestTimeout, () => {
           req.close();
-          const timeoutError = new Error(`Stream timed out because of no activity for ${requestTimeout} ms`);
+          const timeoutError = new Error(`Stream timed out because of no activity for ${effectiveRequestTimeout} ms`);
           timeoutError.name = "TimeoutError";
           rejectWithDestroy(timeoutError);
         });
@@ -227,7 +231,7 @@ export class NodeHttp2Handler implements HttpHandler<NodeHttp2HandlerOptions> {
         }
       });
 
-      writeRequestBodyPromise = writeRequestBody(req, request, requestTimeout);
+      writeRequestBodyPromise = writeRequestBody(req, request, effectiveRequestTimeout);
     });
   }