Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace socketTimeout with bodyTimeout #460

Merged
merged 24 commits into from
Oct 26, 2020
20 changes: 6 additions & 14 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down Expand Up @@ -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`

Expand All @@ -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.
Expand Down Expand Up @@ -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. |
Expand Down
3 changes: 2 additions & 1 deletion benchmarks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ const httpOptions = {
const undiciOptions = {
path: '/',
method: 'GET',
requestTimeout: 0
headersTimeout: 0,
bodyTimeout: 0
}

const client = new Client(`http://${httpOptions.hostname}`, {
Expand Down
74 changes: 19 additions & 55 deletions lib/core/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -35,7 +33,7 @@ const {
kNeedDrain,
kTLSServerName,
kIdleTimeout,
kSocketTimeout,
kHostHeader,
kTLSOpts,
kClosed,
kDestroyed,
Expand All @@ -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])
Expand All @@ -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')
}
Expand Down Expand Up @@ -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')
}
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand All @@ -372,7 +359,7 @@ class Parser extends HTTPParser {
{},
client[kMaxHeadersSize],
insecureHTTPParser,
client[kHeadersTimeout]
0
)
} else {
super(HTTPParser.RESPONSE, false)
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -766,7 +742,6 @@ function detachSocket (socket) {

function connect (client) {
assert(!client[kSocket])
assert(!client[kRetryTimeout])

const { protocol, port, hostname } = client[kUrl]

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down
23 changes: 6 additions & 17 deletions lib/core/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
}

Expand Down Expand Up @@ -140,9 +130,8 @@ class NotSupportedError extends UndiciError {

module.exports = {
UndiciError,
SocketTimeoutError,
HeadersTimeoutError,
RequestTimeoutError,
BodyTimeoutError,
ContentLengthMismatchError,
TrailerMismatchError,
InvalidArgumentError,
Expand Down
Loading