DX-1938: update signal parameter to accept function (#1379)

CahidArda · web-flow · commit a8baa3845f1d · 2025-06-24T17:52:47.000+03:00
* feat: update signal parameter to accept function

* fix: add timeout test
diff --git a/pkg/http.test.ts b/pkg/http.test.ts
@@ -1,5 +1,9 @@
 import { describe, test, expect } from "bun:test";
 import { Redis } from "../platforms/nodejs";
+import { serve } from "bun";
+
+const MOCK_SERVER_PORT = 8080;
+const SERVER_URL = `http://localhost:${MOCK_SERVER_PORT}`;
 
 describe("http", () => {
   test("should terminate after sleeping 5 times", async () => {
@@ -20,4 +24,41 @@ describe("http", () => {
     // if the Promise.race doesn't throw, that means the retries took longer than 4.5s
     expect(throws).toThrow("fetch() URL is invalid");
   });
+
+  test("should throw on request timeouts", async () => {
+    const server = serve({
+      async fetch(request) {
+        const body = await request.text();
+
+        if (body.includes("zed")) {
+          return new Response(JSON.stringify({ result: '"zed-result"' }), { status: 200 });
+        }
+
+        await new Promise((resolve) => setTimeout(resolve, 5000));
+        return new Response("Hello World");
+      },
+      port: MOCK_SERVER_PORT,
+    });
+
+    const redis = new Redis({
+      url: SERVER_URL,
+      token: "non-existent",
+      signal: () => AbortSignal.timeout(1000), // set a timeout of 1 second
+      // set to false since mock server doesn't return a response
+      // for a pipeline. If you want to test pipelining, you can set it to true
+      // and make the mock server return a pipeline response.
+      enableAutoPipelining: false,
+    });
+
+    try {
+      expect(redis.get("foo")).rejects.toThrow("The operation timed out.");
+      expect(redis.get("bar")).rejects.toThrow("The operation timed out.");
+      expect(redis.get("zed")).resolves.toBe("zed-result");
+    } catch (error) {
+      server.stop(true);
+      throw error;
+    } finally {
+      server.stop(true);
+    }
+  });
 });
diff --git a/pkg/http.ts b/pkg/http.ts
@@ -125,7 +125,7 @@ export type HttpClientConfig = {
   options?: Options;
   retry?: RetryConfig;
   agent?: any;
-  signal?: AbortSignal;
+  signal?: AbortSignal | (() => AbortSignal);
   keepAlive?: boolean;
 
   /**
@@ -141,7 +141,7 @@ export class HttpClient implements Requester {
   public readonly options: {
     backend?: string;
     agent: any;
-    signal?: AbortSignal;
+    signal?: HttpClientConfig["signal"];
     responseEncoding?: false | "base64";
     cache?: CacheSetting;
     keepAlive: boolean;
@@ -218,6 +218,9 @@ export class HttpClient implements Requester {
     const requestUrl = [this.baseUrl, ...(req.path ?? [])].join("/");
     const isEventStream = requestHeaders.Accept === "text/event-stream";
 
+    const signal = req.signal ?? this.options.signal;
+    const isSignalFunction = typeof signal === "function";
+
     const requestOptions: RequestInit & { backend?: string; agent?: any } = {
       //@ts-expect-error this should throw due to bun regression
       cache: this.options.cache,
@@ -226,7 +229,7 @@ export class HttpClient implements Requester {
       body: JSON.stringify(req.body),
       keepalive: this.options.keepAlive,
       agent: this.options.agent,
-      signal: req.signal ?? this.options.signal,
+      signal: isSignalFunction ? signal() : signal,
 
       /**
        * Fastly specific
@@ -256,13 +259,15 @@ export class HttpClient implements Requester {
         res = await fetch(requestUrl, requestOptions);
         break;
       } catch (error_) {
-        if (this.options.signal?.aborted) {
+        if (requestOptions.signal?.aborted && isSignalFunction) {
+          throw error_;
+        } else if (requestOptions.signal?.aborted) {
           const myBlob = new Blob([
-            JSON.stringify({ result: this.options.signal.reason ?? "Aborted" }),
+            JSON.stringify({ result: requestOptions.signal.reason ?? "Aborted" }),
           ]);
           const myOptions = {
             status: 200,
-            statusText: this.options.signal.reason ?? "Aborted",
+            statusText: requestOptions.signal.reason ?? "Aborted",
           };
           res = new Response(myBlob, myOptions);
           break;
diff --git a/platforms/cloudflare.ts b/platforms/cloudflare.ts
@@ -1,4 +1,4 @@
-import type { RequesterConfig } from "../pkg/http";
+import type { HttpClientConfig, RequesterConfig } from "../pkg/http";
 import { HttpClient } from "../pkg/http";
 import * as core from "../pkg/redis";
 import { VERSION } from "../version";
@@ -27,7 +27,7 @@ export type RedisConfigCloudflare = {
    * The signal will allow aborting requests on the fly.
    * For more check: https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal
    */
-  signal?: AbortSignal;
+  signal?: HttpClientConfig["signal"];
   keepAlive?: boolean;
 
   /**
diff --git a/platforms/nodejs.ts b/platforms/nodejs.ts
@@ -1,7 +1,7 @@
 /* eslint-disable @typescript-eslint/ban-ts-comment */
 // deno-lint-ignore-file
 
-import type { Requester, RequesterConfig } from "../pkg/http";
+import type { HttpClientConfig, Requester, RequesterConfig } from "../pkg/http";
 import { HttpClient } from "../pkg/http";
 
 import * as core from "../pkg/redis";
@@ -49,7 +49,7 @@ export type RedisConfigNodejs = {
    * The signal will allow aborting requests on the fly.
    * For more check: https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal
    */
-  signal?: AbortSignal;
+  signal?: HttpClientConfig["signal"];
   latencyLogging?: boolean;
   agent?: unknown;
   keepAlive?: boolean;
