From a12a7391fba39b5c114fa658590fae305dcedd17 Mon Sep 17 00:00:00 2001
From: Richard Moore <me@ricmoo.com>
Date: Wed, 24 Apr 2024 23:56:08 -0400
Subject: [PATCH] Added consistent timeout and cancel behaviour to FetchRequest
 (#4122).

---
 src.ts/utils/geturl-browser.ts | 36 +++++++++++++++++++++++++---------
 src.ts/utils/geturl.ts         | 24 ++++++++++++++++++++---
 2 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/src.ts/utils/geturl-browser.ts b/src.ts/utils/geturl-browser.ts
index a71abb2553..f57bc020e8 100644
--- a/src.ts/utils/geturl-browser.ts
+++ b/src.ts/utils/geturl-browser.ts
@@ -1,4 +1,4 @@
-import { assert } from "./errors.js";
+import { assert, makeError } from "./errors.js";
 
 import type {
     FetchGetUrlFunc, FetchRequest, FetchCancelSignal, GetUrlResponse
@@ -27,11 +27,11 @@ declare global {
     function fetch(url: string, init: FetchInit): Promise<Response>;
 }
 
-// @TODO: timeout is completely ignored; start a Promise.any with a reject?
-
 export function createGetUrl(options?: Record<string, any>): FetchGetUrlFunc {
 
     async function getUrl(req: FetchRequest, _signal?: FetchCancelSignal): Promise<GetUrlResponse> {
+        assert(signal == null || !signal.cancelled, "request cancelled before sending", "CANCELLED");
+
         const protocol = req.url.split(":")[0].toLowerCase();
 
         assert(protocol === "http" || protocol === "https", `unsupported protocol ${ protocol }`, "UNSUPPORTED_OPERATION", {
@@ -43,21 +43,39 @@ export function createGetUrl(options?: Record<string, any>): FetchGetUrlFunc {
             operation: "request"
         });
 
-        let signal: undefined | AbortSignal = undefined;
+        let error: null | Error = null;
+
+        const controller = new AbortController();
+
+        const timer = setTimeout(() => {
+            error = makeError("request timeout", "TIMEOUT");
+            controller.abort();
+        }, req.timeout);
+
         if (_signal) {
-            const controller = new AbortController();
-            signal = controller.signal;
-            _signal.addListener(() => { controller.abort(); });
+            _signal.addListener(() => {
+                error = makeError("request cancelled", "CANCELLED");
+                controller.abort();
+            });
         }
 
         const init = {
             method: req.method,
             headers: new Headers(Array.from(req)),
             body: req.body || undefined,
-            signal
+            signal: controller.signal
         };
 
-        const resp = await fetch(req.url, init);
+        let resp: Awaited<ReturnType<typeof fetch>>;
+        try {
+            resp = await fetch(req.url, init);
+        } catch (_error) {
+            clearTimeout(timer);
+            if (error) { throw error; }
+            throw _error;
+        }
+
+        clearTimeout(timer);
 
         const headers: Record<string, string> = { };
         resp.headers.forEach((value, key) => {
diff --git a/src.ts/utils/geturl.ts b/src.ts/utils/geturl.ts
index b7220acd24..571eaaa731 100644
--- a/src.ts/utils/geturl.ts
+++ b/src.ts/utils/geturl.ts
@@ -2,7 +2,7 @@ import http from "http";
 import https from "https";
 import { gunzipSync } from "zlib";
 
-import { assert } from "./errors.js";
+import { assert, makeError } from "./errors.js";
 import { getBytes } from "./data.js";
 
 import type {
@@ -15,6 +15,8 @@ import type {
 export function createGetUrl(options?: Record<string, any>): FetchGetUrlFunc {
 
     async function getUrl(req: FetchRequest, signal?: FetchCancelSignal): Promise<GetUrlResponse> {
+        // Make sure we weren't cancelled before sending
+        assert(signal == null || !signal.cancelled, "request cancelled before sending", "CANCELLED");
 
         const protocol = req.url.split(":")[0].toLowerCase();
 
@@ -35,6 +37,13 @@ export function createGetUrl(options?: Record<string, any>): FetchGetUrlFunc {
             if (options.agent) { reqOptions.agent = options.agent; }
         }
 
+        // Create a Node-specific AbortController, if available
+        let abort: null | AbortController = null;
+        try {
+            abort = new AbortController();
+            reqOptions.abort = abort.signal;
+        } catch (e) { console.log(e); }
+
         const request = ((protocol === "http") ? http: https).request(req.url, reqOptions);
 
         request.setTimeout(req.timeout);
@@ -45,8 +54,17 @@ export function createGetUrl(options?: Record<string, any>): FetchGetUrlFunc {
         request.end();
 
         return new Promise((resolve, reject) => {
-            // @TODO: Node 15 added AbortSignal; once we drop support for
-            // Node14, we can add that in here too
+
+            if (signal) {
+                signal.addListener(() => {
+                    if (abort) { abort.abort(); }
+                    reject(makeError("request cancelled", "CANCELLED"));
+                });
+            }
+
+            request.on("timeout", () => {
+                reject(makeError("request timeout", "TIMEOUT"));
+            });
 
             request.once("response", (resp: http.IncomingMessage) => {
                 const statusCode = resp.statusCode || 0;