fix: enforce timeout while reading response bodies

src/core.ts

@@ -54,41 +54,64 @@ type APIResponseProps = {
   response: Response;
   options: FinalRequestOptions;
   controller: AbortController;
+  timeout: number;
+  requestStartedAt: number;
 };
 
 async function defaultParseResponse<T>(props: APIResponseProps): Promise<T> {
-  const { response } = props;
-  // fetch refuses to read the body when the status code is 204.
-  if (response.status === 204) {
-    return null as T;
-  }
-
-  if (props.options.__binaryResponse) {
-    return response as unknown as T;
-  }
+  return readResponseBodyWithTimeout(props, async () => {
+    const { response } = props;
+    // fetch refuses to read the body when the status code is 204.
+    if (response.status === 204) {
+      return null as T;
+    }
 
-  const contentType = response.headers.get('content-type');
-  const mediaType = contentType?.split(';')[0]?.trim();
-  const isJSON = mediaType?.includes('application/json') || mediaType?.endsWith('+json');
-  if (isJSON) {
-    const contentLength = response.headers.get('content-length');
-    if (contentLength === '0') {
-      // if there is no content we can't do anything
-      return undefined as T;
+    if (props.options.__binaryResponse) {
+      return response as unknown as T;
     }
 
-    const json = await response.json();
+    const contentType = response.headers.get('content-type');
+    const mediaType = contentType?.split(';')[0]?.trim();
+    const isJSON = mediaType?.includes('application/json') || mediaType?.endsWith('+json');
+    if (isJSON) {
+      const contentLength = response.headers.get('content-length');
+      if (contentLength === '0') {
+        // if there is no content we can't do anything
+        return undefined as T;
+      }
 
-    debug('response', response.status, response.url, response.headers, json);
+      const json = await response.json();
 
-    return json as T;
-  }
+      debug('response', response.status, response.url, response.headers, json);
+
+      return json as T;
+    }
 
-  const text = await response.text();
-  debug('response', response.status, response.url, response.headers, text);
+    const text = await response.text();
+    debug('response', response.status, response.url, response.headers, text);
 
-  // TODO handle blob, arraybuffer, other content types, etc.
-  return text as unknown as T;
+    // TODO handle blob, arraybuffer, other content types, etc.
+    return text as unknown as T;
+  });
+}
+
+async function readResponseBodyWithTimeout<T>(
+  props: APIResponseProps,
+  readBody: () => Promise<T>,
+): Promise<T> {
+  const remaining = Math.max(props.timeout - (Date.now() - props.requestStartedAt), 0);
+  const timeoutId = setTimeout(() => props.controller.abort(), remaining);
+  try {
+    return await readBody();
+  } catch (err) {
+    if (props.controller.signal.aborted) {
+      if (props.options.signal?.aborted) throw new APIUserAbortError();
+      throw new APIConnectionTimeoutError();
+    }
+    throw err;
+  } finally {
+    clearTimeout(timeoutId);
+  }
 }
 
 /**
@@ -470,6 +493,7 @@ export abstract class APIClient {
     }
 
     const controller = new AbortController();
+    const requestStartedAt = Date.now();
     const response = await this.fetchWithTimeout(url, req, timeout, controller).catch(castToError);
 
     if (response instanceof Error) {
@@ -494,7 +518,14 @@ export abstract class APIClient {
         return this.retryRequest(options, retriesRemaining, responseHeaders);
       }
 
-      const errText = await response.text().catch((e) => castToError(e).message);
+      const responseProps = { response, options, controller, timeout, requestStartedAt };
+      let errText: string;
+      try {
+        errText = await readResponseBodyWithTimeout(responseProps, () => response.text());
+      } catch (err) {
+        if (err instanceof APIConnectionTimeoutError || err instanceof APIUserAbortError) throw err;
+        errText = castToError(err).message;
+      }
       const errJSON = safeJSON(errText);
       const errMessage = errJSON ? undefined : errText;
       const retryMessage = retriesRemaining ? `(error; no more retries left)` : `(error; not retryable)`;
@@ -505,7 +536,7 @@ export abstract class APIClient {
       throw err;
     }
 
-    return { response, options, controller };
+    return { response, options, controller, timeout, requestStartedAt };
   }
 
   requestAPIList<Item = unknown, PageClass extends AbstractPage<Item> = AbstractPage<Item>>(

tests/index.test.ts

@@ -148,6 +148,59 @@ describe('instantiate client', () => {
     expect(capturedRequest?.method).toEqual('PATCH');
   });
 
+  describe('response body timeout', () => {
+    const makeBodyResponse = (
+      signal: RequestInit['signal'],
+      { contentType, status = 200 }: { contentType: string; status?: number },
+    ): Response => {
+      const readBody = () =>
+        new Promise<never>((_resolve, reject) => {
+          signal?.addEventListener('abort', () => {
+            const error = new Error('aborted');
+            error.name = 'AbortError';
+            reject(error);
+          });
+        });
+
+      return {
+        headers: new Response(undefined, { headers: { 'Content-Type': contentType } }).headers,
+        json: readBody,
+        ok: status >= 200 && status < 300,
+        status,
+        text: readBody,
+        url: 'https://example.com/stalled',
+      } as unknown as Response;
+    };
+
+    test('allows a response body to complete before the deadline', async () => {
+      const client = new Browserbase({
+        apiKey: 'My API Key',
+        timeout: 100,
+        fetch: async () =>
+          new Response(JSON.stringify({ ok: true }), {
+            headers: { 'Content-Type': 'application/json' },
+          }),
+      });
+
+      await expect(client.get('/foo')).resolves.toEqual({ ok: true });
+    });
+
+    test.each([
+      ['JSON', 'application/json', 200],
+      ['text', 'text/plain', 200],
+      ['error', 'text/plain', 500],
+    ])('times out while reading a stalled %s response body', async (_name, contentType, status) => {
+      const client = new Browserbase({
+        apiKey: 'My API Key',
+        timeout: 10,
+        maxRetries: 0,
+        fetch: async (_url, init) => makeBodyResponse(init?.signal, { contentType, status }),
+      });
+
+      await expect(client.get('/foo')).rejects.toThrow(Browserbase.APIConnectionTimeoutError);
+    });
+  });
+
   describe('baseUrl', () => {
     test('trailing slash', () => {
       const client = new Browserbase({ baseURL: 'http://localhost:5000/custom/path/', apiKey: 'My API Key' });