fix: replace socketTimeout with bodyTimeout (#460)

README.md

@@ -46,10 +46,6 @@ It should only include the protocol, hostname, and the port.
 
 Options:
 
-- `socketTimeout: Number`, the timeout after which a socket with active requests
- will time out. Monitors time between activity on a connected socket.
- Use `0` to disable it entirely. Default: `30e3` milliseconds (30s).
-
 - `socketPath: String|Null`, an IPC endpoint, either Unix domain socket or Windows named pipe.
  Default: `null`.
 
@@ -82,10 +78,6 @@ Options:
 - `maxHeaderSize: Number`, the maximum length of request headers in bytes.
  Default: `16384` (16KiB).
 
-- `headersTimeout: Number`, the amount of time the parser will wait to receive the complete
- HTTP headers (Node 14 and above only).
- Default: `30e3` milliseconds (30s).
-
 <a name='request'></a>
 #### `client.request(opts[, callback(err, data)]): Promise|Void`
 
@@ -102,10 +94,12 @@ Options:
  Default: `null`.
 * `signal: AbortController|EventEmitter|Null`
  Default: `null`.
-* `requestTimeout: Number`, the timeout after which a request will time out, in
- milliseconds. Monitors time between request being enqueued and receiving
- a response. Use `0` to disable it entirely.
- Default: `30e3` milliseconds (30s).
+- `headersTimeout: Number`, the timeout after which a request will time out, in
+ milliseconds. Monitors time between receiving a complete headers. 
+ Use `0` to disable it entirely. Default: `30e3` milliseconds (30s).
+- `bodyTimeout: Number`, the timeout after which a request will time out, in
+ milliseconds. Monitors time between receiving a body data. 
+ Use `0` to disable it entirely. Default: `30e3` milliseconds (30s).
 * `idempotent: Boolean`, whether the requests can be safely retried or not.
  If `false` the request won't be sent until all preceeding
  requests in the pipeline has completed.
@@ -604,8 +598,6 @@ const { errors } = require('undici')
 | -----------------------------|-----------------------------------|------------------------------------------------|
 | `InvalidArgumentError` | `UND_ERR_INVALID_ARG` | passed an invalid argument. |
 | `InvalidReturnValueError` | `UND_ERR_INVALID_RETURN_VALUE` | returned an invalid value. |
-| `SocketTimeoutError` | `UND_ERR_SOCKET_TIMEOUT` | a socket exceeds the `socketTimeout` option. |
-| `RequestTimeoutError` | `UND_ERR_REQUEST_TIMEOUT` | a request exceeds the `requestTimeout` option. |
 | `RequestAbortedError` | `UND_ERR_ABORTED` | the request has been aborted by the user |
 | `ClientDestroyedError` | `UND_ERR_DESTROYED` | trying to use a destroyed client. |
 | `ClientClosedError` | `UND_ERR_CLOSED` | trying to use a closed client. |

benchmarks/index.js

@@ -25,7 +25,8 @@ const httpOptions = {
 const undiciOptions = {
  path: '/',
  method: 'GET',
- requestTimeout: 0
+ headersTimeout: 0,
+ bodyTimeout: 0
 }
 
 const client = new Client(`https://${httpOptions.hostname}`, {

lib/core/client.js

@@ -10,13 +10,11 @@ const util = require('./util')
 const Request = require('./request')
 const {
  ContentLengthMismatchError,
- SocketTimeoutError,
  TrailerMismatchError,
  InvalidArgumentError,
  RequestAbortedError,
  ClientDestroyedError,
  ClientClosedError,
- HeadersTimeoutError,
  SocketError,
  InformationalError
 } = require('./errors')
@@ -35,7 +33,7 @@ const {
  kNeedDrain,
  kTLSServerName,
  kIdleTimeout,
- kSocketTimeout,
+ kHostHeader,
  kTLSOpts,
  kClosed,
  kDestroyed,
@@ -44,20 +42,15 @@ const {
  kError,
  kOnDestroyed,
  kPipelining,
- kRetryDelay,
- kRetryTimeout,
  kSocket,
  kSocketPath,
  kKeepAliveTimeout,
  kMaxHeadersSize,
- kHeadersTimeout,
  kKeepAliveMaxTimeout,
  kKeepAliveTimeoutThreshold,
  kTLSSession
 } = require('./symbols')
 
-const kHostHeader = Symbol('host header')
-
 const nodeVersions = process.version.split('.')
 const nodeMajorVersion = parseInt(nodeVersions[0].slice(1))
 const nodeMinorVersion = parseInt(nodeVersions[1])
@@ -79,6 +72,14 @@ class Client extends EventEmitter {
  } = {}) {
  super()
 
+ if (socketTimeout !== undefined) {
+ throw new InvalidArgumentError('unsupported socketTimeout')
+ }
+
+ if (headersTimeout !== undefined) {
+ throw new InvalidArgumentError('unsupported headersTimeout')
+ }
+
  if (idleTimeout !== undefined) {
  throw new InvalidArgumentError('unsupported idleTimeout, use keepAliveTimeout instead')
  }
@@ -119,10 +120,6 @@ class Client extends EventEmitter {
  throw new InvalidArgumentError('invalid maxHeaderSize')
  }
 
- if (socketTimeout != null && !Number.isFinite(socketTimeout)) {
- throw new InvalidArgumentError('invalid socketTimeout')
- }
-
  if (keepAliveTimeout != null && (!Number.isFinite(keepAliveTimeout) || keepAliveTimeout <= 0)) {
  throw new InvalidArgumentError('invalid keepAliveTimeout')
  }
@@ -135,18 +132,12 @@ class Client extends EventEmitter {
  throw new InvalidArgumentError('invalid keepAliveTimeoutThreshold')
  }
 
- if (headersTimeout != null && !Number.isFinite(headersTimeout)) {
- throw new InvalidArgumentError('invalid headersTimeout')
- }
-
  this[kSocket] = null
  this[kReset] = false
  this[kPipelining] = pipelining != null ? pipelining : 1
  this[kMaxHeadersSize] = maxHeaderSize || 16384
- this[kHeadersTimeout] = headersTimeout == null ? 30e3 : headersTimeout
  this[kUrl] = url
  this[kSocketPath] = socketPath
- this[kSocketTimeout] = socketTimeout == null ? 30e3 : socketTimeout
  this[kIdleTimeout] = keepAliveTimeout == null ? 4e3 : keepAliveTimeout
  this[kKeepAliveMaxTimeout] = keepAliveMaxTimeout == null ? 600e3 : keepAliveMaxTimeout
  this[kKeepAliveTimeoutThreshold] = keepAliveTimeoutThreshold == null ? 1e3 : keepAliveTimeoutThreshold
@@ -156,8 +147,6 @@ class Client extends EventEmitter {
  this[kTLSServerName] = (tls && tls.servername) || null
  this[kHost] = null
  this[kTLSOpts] = tls
- this[kRetryDelay] = 0
- this[kRetryTimeout] = null
  this[kOnDestroyed] = []
  this[kWriting] = false
  this[kResuming] = 0 // 0, idle, 1, scheduled, 2 resuming
@@ -323,8 +312,6 @@ class Client extends EventEmitter {
  request.onError(err)
  }
 
- clearTimeout(this[kRetryTimeout])
- this[kRetryTimeout] = null
  this[kClosed] = true
  this[kDestroyed] = true
  this[kOnDestroyed].push(callback)
@@ -355,15 +342,15 @@ class Parser extends HTTPParser {
  this.initialize(
  HTTPParser.RESPONSE,
  {},
- client[kHeadersTimeout]
+ 0
  )
  } else if (nodeMajorVersion === 12 && nodeMinorVersion >= 19) {
  super()
  this.initialize(
  HTTPParser.RESPONSE,
  {},
  client[kMaxHeadersSize],
- client[kHeadersTimeout]
+ 0
  )
  } else if (nodeMajorVersion > 12) {
  super()
@@ -372,7 +359,7 @@ class Parser extends HTTPParser {
  {},
  client[kMaxHeadersSize],
  insecureHTTPParser,
- client[kHeadersTimeout]
+ 0
  )
  } else {
  super(HTTPParser.RESPONSE, false)
@@ -389,15 +376,6 @@ class Parser extends HTTPParser {
  this.request = null
  }
 
- [HTTPParser.kOnTimeout] () {
- /* istanbul ignore next: https://github.com/nodejs/node/pull/34578 */
- if (this.statusCode) {
- this.socket._unrefTimer()
- } else {
- util.destroy(this.socket, new HeadersTimeoutError())
- }
- }
-
  [HTTPParser.kOnHeaders] (rawHeaders) {
  if (this.headers) {
  Array.prototype.push.apply(this.headers, rawHeaders)
@@ -665,16 +643,14 @@ function onSocketConnect () {
 
  assert(!this.destroyed)
  assert(!client[kWriting])
- assert(!client[kRetryTimeout])
 
  client[kReset] = false
- client[kRetryDelay] = 0
  client.emit('connect')
  resume(client)
 }
 
-function onSocketTimeout () {
- util.destroy(this, new SocketTimeoutError())
+function onIdleTimeout () {
+ util.destroy(this, new InformationalError('socket idle timeout'))
 }
 
 function onSocketError (err) {
@@ -757,7 +733,7 @@ function detachSocket (socket) {
  socket[kClient] = null
  socket[kError] = null
  socket
- .removeListener('timeout', onSocketTimeout)
+ .removeListener('timeout', onIdleTimeout)
  .removeListener('session', onSocketSession)
  .removeListener('error', onSocketError)
  .removeListener('end', onSocketEnd)
@@ -766,7 +742,6 @@ function detachSocket (socket) {
 
 function connect (client) {
  assert(!client[kSocket])
- assert(!client[kRetryTimeout])
 
  const { protocol, port, hostname } = client[kUrl]
 
@@ -812,7 +787,7 @@ function connect (client) {
  .setNoDelay(true)
  .setTimeout(client[kIdleTimeout])
  .on(protocol === 'https:' ? 'secureConnect' : 'connect', onSocketConnect)
- .on('timeout', onSocketTimeout)
+ .on('timeout', onIdleTimeout)
  .on('error', onSocketError)
  .on('end', onSocketEnd)
  .on('close', onSocketClose)
@@ -874,9 +849,7 @@ function _resume (client, sync) {
  }
 
  if (client[kSocket]) {
- const timeout = client.running
- ? client[kSocketTimeout]
- : client[kKeepAliveTimeout]
+ const timeout = client.running ? 0 : client[kKeepAliveTimeout]
 
  if (client[kSocket].timeout !== timeout) {
  client[kSocket].setTimeout(timeout)
@@ -936,17 +909,8 @@ function _resume (client, sync) {
  }
  }
 
- if (!client[kSocket] && !client[kRetryTimeout]) {
- if (client[kRetryDelay]) {
- client[kRetryTimeout] = setTimeout(() => {
- client[kRetryTimeout] = null
- connect(client)
- }, client[kRetryDelay])
- client[kRetryDelay] = Math.min(client[kRetryDelay] * 2, client[kSocketTimeout])
- } else {
- connect(client)
- client[kRetryDelay] = 1e3
- }
+ if (!client[kSocket]) {
+ connect(client)
  return
  }
 

lib/core/errors.js

@@ -18,23 +18,13 @@ class HeadersTimeoutError extends UndiciError {
  }
 }
 
-class SocketTimeoutError extends UndiciError {
+class BodyTimeoutError extends UndiciError {
  constructor (message) {
  super(message)
- Error.captureStackTrace(this, SocketTimeoutError)
- this.name = 'SocketTimeoutError'
- this.message = message || 'Socket Timeout Error'
- this.code = 'UND_ERR_SOCKET_TIMEOUT'
- }
-}
-
-class RequestTimeoutError extends UndiciError {
- constructor (message) {
- super(message)
- Error.captureStackTrace(this, RequestTimeoutError)
- this.name = 'RequestTimeoutError'
- this.message = message || 'Request Timeout Error'
- this.code = 'UND_ERR_REQUEST_TIMEOUT'
+ Error.captureStackTrace(this, BodyTimeoutError)
+ this.name = 'BodyTimeoutError'
+ this.message = message || 'Body Timeout Error'
+ this.code = 'UND_ERR_BODY_TIMEOUT'
  }
 }
 
@@ -140,9 +130,8 @@ class NotSupportedError extends UndiciError {
 
 module.exports = {
  UndiciError,
- SocketTimeoutError,
  HeadersTimeoutError,
- RequestTimeoutError,
+ BodyTimeoutError,
  ContentLengthMismatchError,
  TrailerMismatchError,
  InvalidArgumentError,