From a8baa3845f1dbfa1e2fee842f3e99ce6a3492d2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cahid=20Arda=20=C3=96z?= Date: Tue, 24 Jun 2025 17:52:47 +0300 Subject: [PATCH] DX-1938: update signal parameter to accept function (#1379) * feat: update signal parameter to accept function * fix: add timeout test --- pkg/http.test.ts | 41 +++++++++++++++++++++++++++++++++++++++++ pkg/http.ts | 17 +++++++++++------ platforms/cloudflare.ts | 4 ++-- platforms/nodejs.ts | 4 ++-- 4 files changed, 56 insertions(+), 10 deletions(-) diff --git a/pkg/http.test.ts b/pkg/http.test.ts index 38d2122b..93c5d5f4 100644 --- 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 index 3a0ba7f5..8f6e47b0 100644 --- 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 index c62e4920..4bf620e2 100644 --- 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 index 71311ee8..5ed6dcf8 100644 --- 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;