Remove default 30-second timeout on HTTP requests (#201)

* Don't alias setTimeout fn

* Clean up tests

Use more promises, nest tests into more suites

* Remove default 30s timeout on HTTP requests

src/Transport.ts

@@ -81,7 +81,7 @@ import {
   kRetryBackoff,
   kOtelTracer
 } from './symbols'
-import { setTimeout as setTimeoutPromise } from 'node:timers/promises'
+import { setTimeout } from 'node:timers/promises'
 import opentelemetry, { Attributes, Exception, SpanKind, SpanStatusCode, Span, Tracer } from '@opentelemetry/api'
 
 const { version: clientVersion } = require('../package.json') // eslint-disable-line
@@ -139,7 +139,7 @@ export interface TransportRequestParams {
 
 export interface TransportRequestOptions {
   ignore?: number[]
-  requestTimeout?: number | string
+  requestTimeout?: number | string | null
   retryOnTimeout?: boolean
   maxRetries?: number
   asStream?: boolean
@@ -214,7 +214,7 @@ export default class Transport {
   [kName]: string | symbol
   [kMaxRetries]: number
   [kCompression]: boolean
-  [kRequestTimeout]: number
+  [kRequestTimeout]: number | null
   [kRetryOnTimeout]: boolean
   [kSniffEnabled]: boolean
   [kNextSniff]: number
@@ -277,7 +277,7 @@ export default class Transport {
     this[kName] = opts.name ?? 'elastic-transport-js'
     this[kMaxRetries] = typeof opts.maxRetries === 'number' ? opts.maxRetries : 3
     this[kCompression] = opts.compression === true
-    this[kRequestTimeout] = opts.requestTimeout != null ? toMs(opts.requestTimeout) : 30000
+    this[kRequestTimeout] = opts.requestTimeout != null ? toMs(opts.requestTimeout) : null
     this[kRetryOnTimeout] = opts.retryOnTimeout != null ? opts.retryOnTimeout : false
     this[kSniffInterval] = opts.sniffInterval ?? false
     this[kSniffEnabled] = typeof this[kSniffInterval] === 'number'
@@ -517,6 +517,10 @@ export default class Transport {
 
         this[kDiagnostic].emit('request', null, result)
 
+        // set timeout defaults
+        let timeout = options.requestTimeout ?? this[kRequestTimeout] ?? undefined
+        if (timeout != null) timeout = toMs(timeout)
+
         // perform the actual http request
         let { statusCode, headers, body } = await meta.connection.request(connectionParams, {
           requestId: meta.request.id,
@@ -525,7 +529,7 @@ export default class Transport {
           maxResponseSize,
           maxCompressedResponseSize,
           signal,
-          timeout: toMs(options.requestTimeout != null ? options.requestTimeout : this[kRequestTimeout]),
+          timeout,
           ...(options.asStream === true ? { asStream: true } : null)
         })
         result.statusCode = statusCode
@@ -657,7 +661,7 @@ export default class Transport {
                 const backoff = options.retryBackoff ?? this[kRetryBackoff]
                 const backoffWait = backoff(0, 4, meta.attempts)
                 if (backoffWait > 0) {
-                  await setTimeoutPromise(backoffWait * 1000)
+                  await setTimeout(backoffWait * 1000)
                 }
               }
 

src/connection/BaseConnection.ts

@@ -42,7 +42,7 @@ export interface ConnectionOptions {
   status?: string
   auth?: BasicAuth | ApiKeyAuth | BearerAuth
   diagnostic?: Diagnostic
-  timeout?: number
+  timeout?: number | null
   agent?: HttpAgentOptions | UndiciAgentOptions | agentFn | boolean
   proxy?: string | URL
   caFingerprint?: string
@@ -64,7 +64,7 @@ export interface ConnectionRequestOptions {
   maxResponseSize?: number
   maxCompressedResponseSize?: number
   signal?: AbortSignal
-  timeout?: number
+  timeout?: number | null
 }
 
 export interface ConnectionRequestOptionsAsStream extends ConnectionRequestOptions {
@@ -90,7 +90,7 @@ export default class BaseConnection {
   url: URL
   tls: TlsConnectionOptions | null
   id: string
-  timeout: number
+  timeout: number | null
   headers: http.IncomingHttpHeaders
   deadCount: number
   resurrectTimeout: number
@@ -111,6 +111,7 @@ export default class BaseConnection {
     this.tls = opts.tls ?? null
     this.id = opts.id ?? stripAuth(opts.url.href)
     this.headers = prepareHeaders(opts.headers, opts.auth)
+    this.timeout = opts.timeout ?? null
     this.timeout = opts.timeout ?? 30000
     this.deadCount = 0
     this.resurrectTimeout = 0

src/connection/HttpConnection.ts

@@ -44,7 +44,7 @@ import {
   RequestAbortedError,
   TimeoutError
 } from '../errors'
-import { setTimeout as setTimeoutPromise } from 'node:timers/promises'
+import { setTimeout } from 'node:timers/promises'
 import { HttpAgentOptions } from '../types'
 
 const debug = Debug('elasticsearch')
@@ -316,7 +316,7 @@ export default class HttpConnection extends BaseConnection {
   async close (): Promise<void> {
     debug('Closing connection', this.id)
     while (this._openRequests > 0) {
-      await setTimeoutPromise(1000)
+      await setTimeout(1000)
     }
     /* istanbul ignore else */
     if (this.agent !== undefined) {
@@ -328,7 +328,7 @@ export default class HttpConnection extends BaseConnection {
     const url = this.url
     let search = url.search
     let pathname = url.pathname
-    const request = {
+    const request: http.ClientRequestArgs = {
       protocol: url.protocol,
       hostname: url.hostname[0] === '['
         ? url.hostname.slice(1, -1)
@@ -338,7 +338,9 @@ export default class HttpConnection extends BaseConnection {
       port: url.port !== '' ? url.port : undefined,
       headers: this.headers,
       agent: this.agent,
-      timeout: options.timeout ?? this.timeout
+      // only set a timeout if it has a value; default to no timeout
+      // see https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-network.html#_http_client_configuration
+      timeout: options.timeout ?? this.timeout ?? undefined
     }
 
     const paramsKeys = Object.keys(params)

src/connection/UndiciConnection.ts

@@ -77,8 +77,10 @@ export default class Connection extends BaseConnection {
       pipelining: 1,
       maxHeaderSize: 16384,
       connections: 256,
-      headersTimeout: this.timeout,
-      bodyTimeout: this.timeout,
+      // only set a timeout if it has a value; default to no timeout
+      // see https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-network.html#_http_client_configuration
+      headersTimeout: this.timeout ?? undefined,
+      bodyTimeout: this.timeout ?? undefined,
       ...opts.agent
     }