fix: set socketErrorRetry = 1 by default (#455)

closes #454

README.md

@@ -64,16 +64,17 @@ console.log('status: %s, body size: %d, headers: %j', res.status, data.length, r
   - ***dataType*** String - Type of response data. Could be `text` or `json`. If it's `text`, the `callback`ed `data` would be a String. If it's `json`, the `data` of callback would be a parsed JSON Object and will auto set `Accept: application/json` header. Default `callback`ed `data` would be a `Buffer`.
   - **fixJSONCtlChars** Boolean - Fix the control characters (U+0000 through U+001F) before JSON parse response. Default is `false`.
   - ***headers*** Object - Request headers.
-  - ***timeout*** Number | Array - Request timeout in milliseconds for connecting phase and response receiving phase. Defaults to `exports.TIMEOUT`, both are 5s. You can use `timeout: 5000` to tell urllib use same timeout on two phase or set them seperately such as `timeout: [3000, 5000]`, which will set connecting timeout to 3s and response 5s.
+  - ***timeout*** Number | Array - Request timeout in milliseconds for connecting phase and response receiving phase. Default is `5000`. You can use `timeout: 5000` to tell urllib use same timeout on two phase or set them seperately such as `timeout: [3000, 5000]`, which will set connecting timeout to 3s and response 5s.
+  - **keepAliveTimeout** `number | null` - Default is `4000`, 4 seconds - The timeout after which a socket without active requests will time out. Monitors time between activity on a connected socket. This value may be overridden by *keep-alive* hints from the server. See [MDN: HTTP - Headers - Keep-Alive directives](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Keep-Alive#directives) for more details.
   - ***auth*** String - `username:password` used in HTTP Basic Authorization.
   - ***digestAuth*** String - `username:password` used in HTTP [Digest Authorization](https://en.wikipedia.org/wiki/Digest_access_authentication).
   - ***followRedirect*** Boolean - follow HTTP 3xx responses as redirects. defaults to false.
   - ***maxRedirects*** Number - The maximum number of redirects to follow, defaults to 10.
   - ***formatRedirectUrl*** Function - Format the redirect url by your self. Default is `url.resolve(from, to)`.
   - ***beforeRequest*** Function - Before request hook, you can change every thing here.
   - ***streaming*** Boolean - let you get the `res` object when request  connected, default `false`. alias `customResponse`
-  - ***compressed*** Boolean - Accept `gzip, br` response content and auto decode it, default is `false`.
-  - ***timing*** Boolean - Enable timing or not, default is `false`.
+  - ***compressed*** Boolean - Accept `gzip, br` response content and auto decode it, default is `true`.
+  - ***timing*** Boolean - Enable timing or not, default is `true`.
   - ***socketPath*** String | null - request a unix socket service, default is `null`.
   - ***highWaterMark*** Number - default is `67108864`, 64 KiB.
 

package.json

@@ -53,7 +53,7 @@
     "build:test": "npm run build && npm run build:cjs:test && npm run build:esm:test && npm run test-tsc",
     "test-tsc": "tsc -p ./test/fixtures/ts/tsconfig.json",
     "test": "npm run lint && vitest run",
-    "test-keepalive": "cross-env TEST_KEEPALIVE_COUNT=50 vitest run --test-timeout 120000 keep-alive-header.test.ts",
+    "test-keepalive": "cross-env TEST_KEEPALIVE_COUNT=50 vitest run --test-timeout 180000 keep-alive-header.test.ts",
     "cov": "vitest run --coverage",
     "ci": "npm run lint && npm run cov && npm run build:test",
     "contributor": "git-contributor",

src/HttpClient.ts

@@ -138,6 +138,7 @@ function defaultIsRetry(response: HttpClientResponse) {
 
 type RequestContext = {
   retries: number;
+  socketErrorRetries: number;
   requestStartTime?: number;
 };
 
@@ -206,6 +207,7 @@ export class HttpClient extends EventEmitter {
     const headers: IncomingHttpHeaders = {};
     const args = {
       retry: 0,
+      socketErrorRetry: 1,
       timing: true,
       ...this.#defaultArgs,
       ...options,
@@ -215,6 +217,7 @@ export class HttpClient extends EventEmitter {
     };
     requestContext = {
       retries: 0,
+      socketErrorRetries: 0,
       ...requestContext,
     };
     if (!requestContext.requestStartTime) {
@@ -281,6 +284,8 @@ export class HttpClient extends EventEmitter {
       requestUrls: [],
       timing,
       socket: socketInfo,
+      retries: requestContext.retries,
+      socketErrorRetries: requestContext.socketErrorRetries,
     } as any as RawResponseWithMeta;
 
     let headersTimeout = 5000;
@@ -324,10 +329,19 @@ export class HttpClient extends EventEmitter {
     if (requestContext.retries > 0) {
       headers['x-urllib-retry'] = `${requestContext.retries}/${args.retry}`;
     }
+    if (requestContext.socketErrorRetries > 0) {
+      headers['x-urllib-retry-on-socket-error'] = `${requestContext.socketErrorRetries}/${args.socketErrorRetry}`;
+    }
     if (args.auth && !headers.authorization) {
       headers.authorization = `Basic ${Buffer.from(args.auth).toString('base64')}`;
     }
 
+    // streaming request should disable socketErrorRetry and retry
+    let isStreamingRequest = false;
+    if (args.dataType === 'stream' || args.writeStream) {
+      isStreamingRequest = true;
+    }
+
     try {
       const requestOptions: IUndiciRequestOption = {
         method,
@@ -356,9 +370,11 @@ export class HttpClient extends EventEmitter {
         if (isReadable(args.stream) && !(args.stream instanceof Readable)) {
           debug('Request#%d convert old style stream to Readable', requestId);
           args.stream = new Readable().wrap(args.stream);
+          isStreamingRequest = true;
         } else if (args.stream instanceof FormStream) {
           debug('Request#%d convert formstream to Readable', requestId);
           args.stream = new Readable().wrap(args.stream);
+          isStreamingRequest = true;
         }
         args.content = args.stream;
       }
@@ -402,6 +418,7 @@ export class HttpClient extends EventEmitter {
           } else if (file instanceof Readable || isReadable(file as any)) {
             const fileName = getFileName(file) || `streamfile${index}`;
             formData.append(field, new BlobFromStream(file, mime.lookup(fileName) || ''), fileName);
+            isStreamingRequest = true;
           }
         }
 
@@ -425,6 +442,7 @@ export class HttpClient extends EventEmitter {
           } else if (typeof args.content === 'string' && !headers['content-type']) {
             headers['content-type'] = 'text/plain;charset=UTF-8';
           }
+          isStreamingRequest = isReadable(args.content);
         }
       } else if (args.data) {
         const isStringOrBufferOrReadable = typeof args.data === 'string'
@@ -441,6 +459,7 @@ export class HttpClient extends EventEmitter {
         } else {
           if (isStringOrBufferOrReadable) {
             requestOptions.body = args.data;
+            isStreamingRequest = isReadable(args.data);
           } else {
             if (args.contentType === 'json'
               || args.contentType === 'application/json'
@@ -456,9 +475,13 @@ export class HttpClient extends EventEmitter {
           }
         }
       }
+      if (isStreamingRequest) {
+        args.retry = 0;
+        args.socketErrorRetry = 0;
+      }
 
-      debug('Request#%d %s %s, headers: %j, headersTimeout: %s, bodyTimeout: %s',
-        requestId, requestOptions.method, requestUrl.href, headers, headersTimeout, bodyTimeout);
+      debug('Request#%d %s %s, headers: %j, headersTimeout: %s, bodyTimeout: %s, isStreamingRequest: %s',
+        requestId, requestOptions.method, requestUrl.href, headers, headersTimeout, bodyTimeout, isStreamingRequest);
       requestOptions.headers = headers;
       channels.request.publish({
         request: reqMeta,
@@ -511,8 +534,6 @@ export class HttpClient extends EventEmitter {
 
       let data: any = null;
       if (args.dataType === 'stream') {
-        // streaming mode will disable retry
-        args.retry = 0;
         // only auto decompress on request args.compressed = true
         if (args.compressed === true && isCompressedContent) {
           // gzip or br
@@ -522,8 +543,6 @@ export class HttpClient extends EventEmitter {
           res = Object.assign(response.body, res);
         }
       } else if (args.writeStream) {
-        // streaming mode will disable retry
-        args.retry = 0;
         if (args.compressed === true && isCompressedContent) {
           const decoder = contentEncoding === 'gzip' ? createGunzip() : createBrotliDecompress();
           await pipelinePromise(response.body, decoder, args.writeStream);
@@ -608,11 +627,8 @@ export class HttpClient extends EventEmitter {
         err = new HttpClientRequestTimeoutError(bodyTimeout, { cause: e });
       } else if (err.code === 'UND_ERR_SOCKET' || err.code === 'ECONNRESET') {
         // auto retry on socket error, https://github.com/node-modules/urllib/issues/454
-        if (args.retry > 0 && requestContext.retries < args.retry) {
-          if (args.retryDelay) {
-            await sleep(args.retryDelay);
-          }
-          requestContext.retries++;
+        if (args.socketErrorRetry > 0 && requestContext.socketErrorRetries < args.socketErrorRetry) {
+          requestContext.socketErrorRetries++;
           return await this.#requestInternal(url, options, requestContext);
         }
       }

src/Request.ts

@@ -69,11 +69,16 @@ export type RequestOptions = {
   headers?: IncomingHttpHeaders;
   /**
    * Request timeout in milliseconds for connecting phase and response receiving phase.
-   * Defaults to exports.
-   * TIMEOUT, both are 5s. You can use timeout: 5000 to tell urllib use same timeout on two phase or set them seperately such as
+   * Defaults is `5000`, both are 5 seconds. You can use timeout: 5000 to tell urllib use same timeout on two phase or set them separately such as
    * timeout: [3000, 5000], which will set connecting timeout to 3s and response 5s.
    */
   timeout?: number | number[];
+  /**
+   * Default is `4000`,  4 seconds - The timeout after which a socket without active requests will time out.
+   * Monitors time between activity on a connected socket.
+   * This value may be overridden by *keep-alive* hints from the server. See [MDN: HTTP - Headers - Keep-Alive directives](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Keep-Alive#directives) for more details.
+   */
+  keepAliveTimeout?: number;
   /**
    * username:password used in HTTP Basic Authorization.
    * Alias to `headers.authorization = xxx`
@@ -91,19 +96,19 @@ export type RequestOptions = {
   formatRedirectUrl?: (a: any, b: any) => void;
   /** Before request hook, you can change every thing here. */
   beforeRequest?: (...args: any[]) => void;
-  /** Accept `gzip, br` response content and auto decode it, default is false. */
+  /** Accept `gzip, br` response content and auto decode it, default is `true`. */
   compressed?: boolean;
   /**
    * @deprecated
    * Alias to compressed
    * */
   gzip?: boolean;
   /**
-   * Enable timing or not, default is true.
+   * Enable timing or not, default is `true`.
    * */
   timing?: boolean;
   /**
-   * Auto retry times on 5xx response, default is 0. Don't work on streaming request
+   * Auto retry times on 5xx response, default is `0`. Don't work on streaming request
    * It's not supported by using retry and writeStream, because the retry request can't stop the stream which is consuming.
    **/
   retry?: number;
@@ -114,6 +119,11 @@ export type RequestOptions = {
    * It will retry when status >= 500 by default. Request error is not included.
    */
   isRetry?: (response: HttpClientResponse) => boolean;
+  /**
+   * Auto retry times on socket error, default is `1`. Don't work on streaming request
+   * It's not supported by using retry and writeStream, because the retry request can't stop the stream which is consuming.
+   **/
+  socketErrorRetry?: number;
   /** Default: `null` */
   opaque?: unknown;
   /**

src/Response.ts

@@ -49,6 +49,8 @@ export type RawResponseWithMeta = Readable & {
   rt: number;
   keepAliveSocket: boolean;
   requestUrls: string[];
+  retries: number;
+  socketErrorRetries: number;
 };
 
 export type HttpClientResponse<T = any> = {

test/HttpClient.events.test.ts

@@ -116,9 +116,9 @@ describe('HttpClient.events.test.ts', () => {
           requestId: 'mock-request-id-1',
         },
         ctx: { foo: 'bar' },
+        socketErrorRetry: 0,
       });
     }, (err: any) => {
-      // console.error(err);
       assert.equal(err.name, 'SocketError');
       assert.equal(err.message, 'other side closed');
       assert.equal(err.status, -1);

test/fixtures/server.ts

@@ -234,6 +234,11 @@ export async function startServer(options?: {
     if (pathname === '/error') {
       return res.destroy();
     }
+    if (pathname === '/error-non-retry') {
+      if (!req.headers['x-urllib-retry-on-socket-error']) {
+        return res.destroy();
+      }
+    }
 
     if (pathname === '/multipart' && (req.method === 'POST' || req.method === 'PUT')) {
       const bb = busboy({ headers: req.headers });

test/keep-alive-header.test.ts

@@ -1,29 +1,22 @@
 import { strict as assert } from 'node:assert';
 import { describe, it, beforeAll, afterAll } from 'vitest';
 import urllib from '../src';
-import { getGlobalDispatcher, setGlobalDispatcher, Agent } from '../src';
 import { startServer } from './fixtures/server';
 import { sleep } from './utils';
 
 describe('keep-alive-header.test.ts', () => {
+  // should shorter than server keepalive timeout
+  // https://zhuanlan.zhihu.com/p/34147188
   const keepAliveTimeout = 2000;
-  const agent = getGlobalDispatcher();
   let close: any;
   let _url: string;
   beforeAll(async () => {
-    // should shorter than server keepalive timeout
-    // https://zhuanlan.zhihu.com/p/34147188
-    const keepalive1sAgent = new Agent({
-      keepAliveTimeout: 1000,
-    });
-    setGlobalDispatcher(keepalive1sAgent);
     const { closeServer, url } = await startServer({ keepAliveTimeout });
     close = closeServer;
     _url = url;
   });
 
   afterAll(async () => {
-    setGlobalDispatcher(agent);
     await close();
   });
 
@@ -32,47 +25,91 @@ describe('keep-alive-header.test.ts', () => {
     const max = process.env.TEST_KEEPALIVE_COUNT ? parseInt(process.env.TEST_KEEPALIVE_COUNT) : 10;
     let otherSideClosed = 0;
     let readECONNRESET = 0;
-    const retry = 1;
     while (count < max) {
       count++;
       try {
-        let response = await urllib.request(_url, { retry });
+        let response = await urllib.request(_url);
+        // console.log(response.res.socket);
         assert.equal(response.status, 200);
         // console.log(response.headers);
         assert.equal(response.headers.connection, 'keep-alive');
         assert.equal(response.headers['keep-alive'], 'timeout=2');
-        response = await urllib.request(_url, { retry });
+        response = await urllib.request(_url);
         assert.equal(response.status, 200);
         // console.log(response.headers);
         assert.equal(response.headers.connection, 'keep-alive');
         assert.equal(response.headers['keep-alive'], 'timeout=2');
-        response = await urllib.request(_url, { retry });
+        response = await urllib.request(_url);
         assert.equal(response.status, 200);
         // console.log(response.headers);
         assert.equal(response.headers.connection, 'keep-alive');
         assert.equal(response.headers['keep-alive'], 'timeout=2');
-        response = await urllib.request(_url, { retry });
+        response = await urllib.request(_url);
         assert.equal(response.status, 200);
         // console.log(response.headers);
         assert.equal(response.headers.connection, 'keep-alive');
         assert.equal(response.headers['keep-alive'], 'timeout=2');
-        response = await urllib.request(_url, { retry });
+        response = await urllib.request(_url);
         assert.equal(response.status, 200);
         assert.equal(response.headers.connection, 'keep-alive');
         assert.equal(response.headers['keep-alive'], 'timeout=2');
-        response = await urllib.request(_url, { retry });
+        response = await urllib.request(_url);
         assert.equal(response.status, 200);
         assert.equal(response.headers.connection, 'keep-alive');
         assert.equal(response.headers['keep-alive'], 'timeout=2');
-        response = await urllib.request(_url, { retry });
+        response = await urllib.request(_url);
         assert.equal(response.status, 200);
         assert.equal(response.headers.connection, 'keep-alive');
         assert.equal(response.headers['keep-alive'], 'timeout=2');
-        response = await urllib.request(_url, { retry });
+        response = await urllib.request(_url);
         assert.equal(response.status, 200);
         assert.equal(response.headers.connection, 'keep-alive');
         assert.equal(response.headers['keep-alive'], 'timeout=2');
-        response = await urllib.request(_url, { retry });
+        response = await urllib.request(_url);
+        assert.equal(response.status, 200);
+        // console.log(response.headers);
+        assert.equal(response.headers.connection, 'keep-alive');
+        assert.equal(response.headers['keep-alive'], 'timeout=2');
+        assert(parseInt(response.headers['x-requests-persocket'] as string) > 1);
+        await sleep(keepAliveTimeout / 2);
+        response = await urllib.request(_url);
+        // console.log(response.res.socket);
+        assert.equal(response.status, 200);
+        // console.log(response.headers);
+        assert.equal(response.headers.connection, 'keep-alive');
+        assert.equal(response.headers['keep-alive'], 'timeout=2');
+        response = await urllib.request(_url);
+        assert.equal(response.status, 200);
+        // console.log(response.headers);
+        assert.equal(response.headers.connection, 'keep-alive');
+        assert.equal(response.headers['keep-alive'], 'timeout=2');
+        response = await urllib.request(_url);
+        assert.equal(response.status, 200);
+        // console.log(response.headers);
+        assert.equal(response.headers.connection, 'keep-alive');
+        assert.equal(response.headers['keep-alive'], 'timeout=2');
+        response = await urllib.request(_url);
+        assert.equal(response.status, 200);
+        // console.log(response.headers);
+        assert.equal(response.headers.connection, 'keep-alive');
+        assert.equal(response.headers['keep-alive'], 'timeout=2');
+        response = await urllib.request(_url);
+        assert.equal(response.status, 200);
+        assert.equal(response.headers.connection, 'keep-alive');
+        assert.equal(response.headers['keep-alive'], 'timeout=2');
+        response = await urllib.request(_url);
+        assert.equal(response.status, 200);
+        assert.equal(response.headers.connection, 'keep-alive');
+        assert.equal(response.headers['keep-alive'], 'timeout=2');
+        response = await urllib.request(_url);
+        assert.equal(response.status, 200);
+        assert.equal(response.headers.connection, 'keep-alive');
+        assert.equal(response.headers['keep-alive'], 'timeout=2');
+        response = await urllib.request(_url);
+        assert.equal(response.status, 200);
+        assert.equal(response.headers.connection, 'keep-alive');
+        assert.equal(response.headers['keep-alive'], 'timeout=2');
+        response = await urllib.request(_url);
         assert.equal(response.status, 200);
         // console.log(response.headers);
         assert.equal(response.headers.connection, 'keep-alive');