Giter VIP home page Giter VIP logo

undici's Introduction

undici

Node CI js-standard-style npm version codecov

An HTTP/1.1 client, written from scratch for Node.js.

Undici means eleven in Italian. 1.1 -> 11 -> Eleven -> Undici. It is also a Stranger Things reference.

How to get involved

Have a question about using Undici? Open a Q&A Discussion or join our official OpenJS Slack channel.

Looking to contribute? Start by reading the contributing guide

Install

npm i undici

Benchmarks

The benchmark is a simple getting data example using a 50 TCP connections with a pipelining depth of 10 running on Node 20.10.0.

Tests Samples Result Tolerance Difference with slowest
undici - fetch 30 3704.43 req/sec ± 2.95 % -
http - no keepalive 20 4275.30 req/sec ± 2.60 % + 15.41 %
node-fetch 10 4759.42 req/sec ± 0.87 % + 28.48 %
request 40 4803.37 req/sec ± 2.77 % + 29.67 %
axios 45 4951.97 req/sec ± 2.88 % + 33.68 %
got 10 5969.67 req/sec ± 2.64 % + 61.15 %
superagent 10 9471.48 req/sec ± 1.50 % + 155.68 %
http - keepalive 25 10327.49 req/sec ± 2.95 % + 178.79 %
undici - pipeline 10 15053.41 req/sec ± 1.63 % + 306.36 %
undici - request 10 19264.24 req/sec ± 1.74 % + 420.03 %
undici - stream 15 20317.29 req/sec ± 2.13 % + 448.46 %
undici - dispatch 10 24883.28 req/sec ± 1.54 % + 571.72 %

The benchmark is a simple sending data example using a 50 TCP connections with a pipelining depth of 10 running on Node 20.10.0.

Tests Samples Result Tolerance Difference with slowest
undici - fetch 20 1968.42 req/sec ± 2.63 % -
http - no keepalive 25 2330.30 req/sec ± 2.99 % + 18.38 %
node-fetch 20 2485.36 req/sec ± 2.70 % + 26.26 %
got 15 2787.68 req/sec ± 2.56 % + 41.62 %
request 30 2805.10 req/sec ± 2.59 % + 42.50 %
axios 10 3040.45 req/sec ± 1.72 % + 54.46 %
superagent 20 3358.29 req/sec ± 2.51 % + 70.61 %
http - keepalive 20 3477.94 req/sec ± 2.51 % + 76.69 %
undici - pipeline 25 3812.61 req/sec ± 2.80 % + 93.69 %
undici - request 10 6067.00 req/sec ± 0.94 % + 208.22 %
undici - stream 10 6391.61 req/sec ± 1.98 % + 224.71 %
undici - dispatch 10 6397.00 req/sec ± 1.48 % + 224.98 %

Quick Start

import { request } from 'undici'

const {
  statusCode,
  headers,
  trailers,
  body
} = await request('http://localhost:3000/foo')

console.log('response received', statusCode)
console.log('headers', headers)

for await (const data of body) { console.log('data', data) }

console.log('trailers', trailers)

Body Mixins

The body mixins are the most common way to format the request/response body. Mixins include:

Note

The body returned from undici.request does not implement .formData().

Example usage:

import { request } from 'undici'

const {
  statusCode,
  headers,
  trailers,
  body
} = await request('http://localhost:3000/foo')

console.log('response received', statusCode)
console.log('headers', headers)
console.log('data', await body.json())
console.log('trailers', trailers)

Note: Once a mixin has been called then the body cannot be reused, thus calling additional mixins on .body, e.g. .body.json(); .body.text() will result in an error TypeError: unusable being thrown and returned through the Promise rejection.

Should you need to access the body in plain-text after using a mixin, the best practice is to use the .text() mixin first and then manually parse the text to the desired format.

For more information about their behavior, please reference the body mixin from the Fetch Standard.

Common API Methods

This section documents our most commonly used API methods. Additional APIs are documented in their own files within the docs folder and are accessible via the navigation list on the left side of the docs site.

undici.request([url, options]): Promise

Arguments:

  • url string | URL | UrlObject
  • options RequestOptions
    • dispatcher Dispatcher - Default: getGlobalDispatcher
    • method String - Default: PUT if options.body, otherwise GET
    • maxRedirections Integer - Default: 0

Returns a promise with the result of the Dispatcher.request method.

Calls options.dispatcher.request(options).

See Dispatcher.request for more details, and request examples for examples.

undici.stream([url, options, ]factory): Promise

Arguments:

  • url string | URL | UrlObject
  • options StreamOptions
    • dispatcher Dispatcher - Default: getGlobalDispatcher
    • method String - Default: PUT if options.body, otherwise GET
    • maxRedirections Integer - Default: 0
  • factory Dispatcher.stream.factory

Returns a promise with the result of the Dispatcher.stream method.

Calls options.dispatcher.stream(options, factory).

See Dispatcher.stream for more details.

undici.pipeline([url, options, ]handler): Duplex

Arguments:

  • url string | URL | UrlObject
  • options PipelineOptions
    • dispatcher Dispatcher - Default: getGlobalDispatcher
    • method String - Default: PUT if options.body, otherwise GET
    • maxRedirections Integer - Default: 0
  • handler Dispatcher.pipeline.handler

Returns: stream.Duplex

Calls options.dispatch.pipeline(options, handler).

See Dispatcher.pipeline for more details.

undici.connect([url, options]): Promise

Starts two-way communications with the requested resource using HTTP CONNECT.

Arguments:

  • url string | URL | UrlObject
  • options ConnectOptions
  • callback (err: Error | null, data: ConnectData | null) => void (optional)

Returns a promise with the result of the Dispatcher.connect method.

Calls options.dispatch.connect(options).

See Dispatcher.connect for more details.

undici.fetch(input[, init]): Promise

Implements fetch.

Basic usage example:

import { fetch } from 'undici'


const res = await fetch('https://example.com')
const json = await res.json()
console.log(json)

You can pass an optional dispatcher to fetch as:

import { fetch, Agent } from 'undici'

const res = await fetch('https://example.com', {
  // Mocks are also supported
  dispatcher: new Agent({
    keepAliveTimeout: 10,
    keepAliveMaxTimeout: 10
  })
})
const json = await res.json()
console.log(json)

request.body

A body can be of the following types:

  • ArrayBuffer
  • ArrayBufferView
  • AsyncIterables
  • Blob
  • Iterables
  • String
  • URLSearchParams
  • FormData

In this implementation of fetch, request.body now accepts Async Iterables. It is not present in the Fetch Standard.

import { fetch } from 'undici'

const data = {
  async *[Symbol.asyncIterator]() {
    yield 'hello'
    yield 'world'
  },
}

await fetch('https://example.com', { body: data, method: 'POST', duplex: 'half' })

FormData besides text data and buffers can also utilize streams via Blob objects:

import { openAsBlob } from 'node:fs'

const file = await openAsBlob('./big.csv')
const body = new FormData()
body.set('file', file, 'big.csv')

await fetch('http://example.com', { method: 'POST', body })

request.duplex

  • half

In this implementation of fetch, request.duplex must be set if request.body is ReadableStream or Async Iterables, however, fetch requests are currently always full duplex. For more detail refer to the Fetch Standard..

response.body

Nodejs has two kinds of streams: web streams, which follow the API of the WHATWG web standard found in browsers, and an older Node-specific streams API. response.body returns a readable web stream. If you would prefer to work with a Node stream you can convert a web stream using .fromWeb().

import { fetch } from 'undici'
import { Readable } from 'node:stream'

const response = await fetch('https://example.com')
const readableWebStream = response.body
const readableNodeStream = Readable.fromWeb(readableWebStream)

Specification Compliance

This section documents parts of the Fetch Standard that Undici does not support or does not fully implement.

Garbage Collection

The Fetch Standard allows users to skip consuming the response body by relying on garbage collection to release connection resources. Undici does not do the same. Therefore, it is important to always either consume or cancel the response body.

Garbage collection in Node is less aggressive and deterministic (due to the lack of clear idle periods that browsers have through the rendering refresh rate) which means that leaving the release of connection resources to the garbage collector can lead to excessive connection usage, reduced performance (due to less connection re-use), and even stalls or deadlocks when running out of connections.

// Do
const headers = await fetch(url)
  .then(async res => {
    for await (const chunk of res.body) {
      // force consumption of body
    }
    return res.headers
  })

// Do not
const headers = await fetch(url)
  .then(res => res.headers)

However, if you want to get only headers, it might be better to use HEAD request method. Usage of this method will obviate the need for consumption or cancelling of the response body. See MDN - HTTP - HTTP request methods - HEAD for more details.

const headers = await fetch(url, { method: 'HEAD' })
  .then(res => res.headers)
Forbidden and Safelisted Header Names

The Fetch Standard requires implementations to exclude certain headers from requests and responses. In browser environments, some headers are forbidden so the user agent remains in full control over them. In Undici, these constraints are removed to give more control to the user.

undici.upgrade([url, options]): Promise

Upgrade to a different protocol. See MDN - HTTP - Protocol upgrade mechanism for more details.

Arguments:

  • url string | URL | UrlObject
  • options UpgradeOptions
  • callback (error: Error | null, data: UpgradeData) => void (optional)

Returns a promise with the result of the Dispatcher.upgrade method.

Calls options.dispatcher.upgrade(options).

See Dispatcher.upgrade for more details.

undici.setGlobalDispatcher(dispatcher)

  • dispatcher Dispatcher

Sets the global dispatcher used by Common API Methods.

undici.getGlobalDispatcher()

Gets the global dispatcher used by Common API Methods.

Returns: Dispatcher

undici.setGlobalOrigin(origin)

  • origin string | URL | undefined

Sets the global origin used in fetch.

If undefined is passed, the global origin will be reset. This will cause Response.redirect, new Request(), and fetch to throw an error when a relative path is passed.

setGlobalOrigin('http://localhost:3000')

const response = await fetch('/api/ping')

console.log(response.url) // http://localhost:3000/api/ping

undici.getGlobalOrigin()

Gets the global origin used in fetch.

Returns: URL

UrlObject

  • port string | number (optional)
  • path string (optional)
  • pathname string (optional)
  • hostname string (optional)
  • origin string (optional)
  • protocol string (optional)
  • search string (optional)

Specification Compliance

This section documents parts of the HTTP/1.1 specification that Undici does not support or does not fully implement.

Expect

Undici does not support the Expect request header field. The request body is always immediately sent and the 100 Continue response will be ignored.

Refs: https://tools.ietf.org/html/rfc7231#section-5.1.1

Pipelining

Undici will only use pipelining if configured with a pipelining factor greater than 1.

Undici always assumes that connections are persistent and will immediately pipeline requests, without checking whether the connection is persistent. Hence, automatic fallback to HTTP/1.0 or HTTP/1.1 without pipelining is not supported.

Undici will immediately pipeline when retrying requests after a failed connection. However, Undici will not retry the first remaining requests in the prior pipeline and instead error the corresponding callback/promise/stream.

Undici will abort all running requests in the pipeline when any of them are aborted.

Manual Redirect

Since it is not possible to manually follow an HTTP redirect on the server-side, Undici returns the actual response instead of an opaqueredirect filtered one when invoked with a manual redirect. This aligns fetch() with the other implementations in Deno and Cloudflare Workers.

Refs: https://fetch.spec.whatwg.org/#atomic-http-redirect-handling

Workarounds

Network address family autoselection.

If you experience problem when connecting to a remote server that is resolved by your DNS servers to a IPv6 (AAAA record) first, there are chances that your local router or ISP might have problem connecting to IPv6 networks. In that case undici will throw an error with code UND_ERR_CONNECT_TIMEOUT.

If the target server resolves to both a IPv6 and IPv4 (A records) address and you are using a compatible Node version (18.3.0 and above), you can fix the problem by providing the autoSelectFamily option (support by both undici.request and undici.Agent) which will enable the family autoselection algorithm when establishing the connection.

Collaborators

Releasers

License

MIT

undici's People

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

undici's Issues

`request.stream` waits for readable side

Not sure if this is a good or bad thing.

The stream we return from the factory function, currently request.stream will wait for the readable side to finish as well if passed a Duplex. I'm not sure whether this is a good or bad thing?

Should `client.destroy()` be able to error?

I'm not sure whether the it makes sense for client.destroy() to error. I haven't really figured out a good use case where it would make sense.

I might want to suggest:

  • Remove err argument from destroy(err).
  • Swallow all errors that occur after the call to destroy(). There will be errors of various kind (depending on timings) from destroying the socket, neither of which makes much sense?

Request Body as Factory Method

Allow passing a factory method as body which would return a Readable. This would allow e.g. idempotent PUT requests which e.g. read from a file to be part of the indirect inflight failure retries.

Alternatively an async generator function which is retryable by definition.

Add opaque to `client.stream`

Right now the user can only pass data into the factory by making the factory a closure which might be inefficient. Add an opaque argument which gets passed to the factory?

e.g.

client.stream({
  path: '/',
  method: 'GET',
  opaque: 'destination.raw'
}, ({ statusCode, headers, opaque: filename }) => {
  console.log('response received', statusCode)
  console.log('headers', headers)
  return fs.createWriteStream(filename)
}, (err) => {
  if (err) {
    console.error('failure', err)
  } else {
    console.log('success')
  }
})
client.stream({
  path: '/',
  method: 'GET',
  opaque: res
}, ({ statusCode, headers, opaque: res }) => {
  console.log('response received', statusCode)
  console.log('headers', headers)
  return res
}, (err) => {
  if (err) {
    console.error('failure', err)
  } else {
    console.log('success')
  }
})

Issue List

This is a meta issue of things left to review and maybe do some tests for:

Improve benchmarks

Would be nice to improve our benchmarks and also maybe add some comparison to our README.

When server response multiple Set-Cookie headers, undici throws TypeError: obj[key].push is not a function

If server response contains multiple set-cookie headers, undici throws following error:

TypeError: obj[key].push is not a function
    at parseHeaders (/Users/wangshuo/Projects/ksite_webapps/node_modules/undici/lib/client.js:341:16)
    at HTTPParser.Client.parser.(anonymous function) (/Users/wangshuo/Projects/ksite_webapps/node_modules/undici/lib/client.js:219:18)
    at HTTPParser.HEADER (/Users/wangshuo/Projects/ksite_webapps/node_modules/http-parser-js/http-parser.js:335:58)
    at HTTPParser.execute (/Users/wangshuo/Projects/ksite_webapps/node_modules/http-parser-js/http-parser.js:111:27)
    at Client.[read] (/Users/wangshuo/Projects/ksite_webapps/node_modules/undici/lib/client.js:268:19)
    at Socket.Client.(anonymous function) (/Users/wangshuo/Projects/ksite_webapps/node_modules/undici/lib/client.js:240:18)
    at Object.onceWrapper (events.js:313:30)
    at ZoneDelegate.invokeTask (/Users/wangshuo/Projects/ksite_webapps/node_modules/zone.js/dist/zone-node.js:421:31)
    at Zone.runTask (/Users/wangshuo/Projects/ksite_webapps/node_modules/zone.js/dist/zone-node.js:188:47)
    at ZoneTask.invokeTask (/Users/wangshuo/Projects/ksite_webapps/node_modules/zone.js/dist/zone-node.js:496:34)
    at Socket.ZoneTask.invoke (/Users/wangshuo/Projects/ksite_webapps/node_modules/zone.js/dist/zone-node.js:485:48)
    at emitNone (events.js:106:13)
    at Socket.emit (events.js:208:7)
    at emitReadable_ (_stream_readable.js:513:10)
    at emitReadable (_stream_readable.js:507:7)
    at addChunk (_stream_readable.js:274:7)
    at readableAddChunk (_stream_readable.js:250:11)
    at Socket.Readable.push (_stream_readable.js:208:10)
    at TCP.onread (net.js:597:20)

request headers mutable?

Currently it is possible for the user to mutate the headers after calling client.request(...). I'm not sure whether this is a good or bad thing?

As an alternative we could stringify the headers object to a rawHeaders buffer/string when creating the request object. That way, if the user modifies the headers object after client.request(...) it won't affect anything.

Thoughts?

Timeout behaviour unclear

The way timeout is currently implement no request may take longer than 30 seconds. Shouldn't the timer be rescheduled every time something happens on the socket (i.e. read/write)?

With the current implementation a large payload over a slow connection will never succeed.

I would have expected timeout be relative to socket activity, not how long a request has been active, or more specifically how long it's been between scheduling of requests.

Client.close w callback or event?

Currently I don't think it's possible to determine when a client has completed all queued requests after calling Client.close() should we add a callback parameter or alternatively emit a 'close' event?

request body 'end' after destroy() asserts

There is currently a race in body request handling when body.'end' is emitted after body.destroy(err) (this is a compat behavior of Readable and of legacy streams) which causes a failure on assert(this.socket) in the finished(body, ...) handler.

client.pipeline API

We recently merged client.pipeline in #147.

Which allows us to do:

stream.pipeline(
  fs.createReadStream('source.raw'),
  client.pipeline({
    path: '/',
    method: 'PUT',
  }, ({ statusCode }) => {
    if (statusCode !== 201) {
      throw new Error('invalid response')
    }
  }),
  fs.createReadStream('response.raw'),
  (err) => {
    if (err) {
      console.error('failed')
    } else {
      console.log('succeeded')
    }
  }
)

However, I was discussing with some collegues the API in regards to whether one would like to modify the stream based on headers. Where the suggestion was to change it to:

stream.pipeline(
  fs.createReadStream('source.raw'),
  client.pipeline({
    path: '/',
    method: 'PUT',
  }, ({ statusCode, headers, body }) => {
    if (statusCode !== 201) {
      throw new Error('invalid response')
    }
    // NOTE: This is different. Receives a `body` and returns a `Readable`.
    // Perform body transformation based on statusCode & headers.
    return isZipped(headers) ? pipeline(body, unzip(), () => {}) : body
  }),
  fs.createReadStream('response.raw'),
  (err) => {
    if (err) {
      console.error('failed')
    } else {
      console.log('succeeded')
    }
  }
)

i.e. the handler receives a body and must return a stream (which usually would simply be body). This makes it more similar with client.stream. If nothing is returned then client.pipeline will automatically and immediately end the returned Duplex.

Makes sense?

Pipelining only for GET/HEAD

Pipelining should only be performed for GET/HEAD requests. If any other requests comes into the queue I guess it should wait for drain and reconnect, or error?

Optimal Pooling Strategy

What is the optimal strategy when picking the next client to perform a request on in the pool?

I believe the follow sorting criteria would be best:

const next = clients
  .sort((a, b) => {
    // connected
    if (a.connected !== b.connected) return a.connected ? -1 : 1
    // queue size
    if (a.size !== b.size) return a.size - b.size
    // drained timestamp
    if (a.drained !== b.drained) return b.drained - a.drained
    return 0
  })

Do we need readable-stream?

Do we need to use readable-stream?

We are using Readable, but given the simplicity of usage I don't believe there should be any problems with it.

Vice-versa with finished which was also included in v10.

throw or callback?

We are a little inconsistent in regards to throwing and using the callback.

For argument errors directly in request, stream and pipeline we throw. While errors from Request constructor we propagate through the callback.

I think we should probably pick one. Should we go with always propagate through callback?

body with GET/HEAD

What should happen if the user supplies a body with a GET/HEAD request? Should we throw an error?

undici doesn't response "curl -I", then if "curl" again, undici will crash

I use fastify-http-proxy with undici. When curl -I http://127.0.0.1:5000/proxy/abc, curl doesn't receive any response (bash keep waiting). Then, if I curl http://127.0.0.1:5000/proxy/abc, which not include -I, undici will crash with following error:

11:19:59 ✨ incoming request HEAD xxx /proxy/abc
11:19:59 ✨ fetching from remote server
11:19:59 ✨ response received
-----> here should be `request completed 25ms`, but missing
11:20:10 ✨ incoming request GET xxx /proxy/abc
11:20:10 ✨ fetching from remote server
TypeError: Cannot read property 'push' of null
    at HTTPParser.Client.parser.(anonymous function) (/Users/wangshuo/Projects/ksite_webapps/node_modules/undici/lib/client.js:229:22)
    at HTTPParser.BODY_RAW (/Users/wangshuo/Projects/ksite_webapps/node_modules/http-parser-js/http-parser.js:408:32)
    at HTTPParser.execute (/Users/wangshuo/Projects/ksite_webapps/node_modules/http-parser-js/http-parser.js:111:27)
    at Client.[read] (/Users/wangshuo/Projects/ksite_webapps/node_modules/undici/lib/client.js:268:19)
    at Socket.Client.(anonymous function) (/Users/wangshuo/Projects/ksite_webapps/node_modules/undici/lib/client.js:240:18)
    at Object.onceWrapper (events.js:313:30)
    at ZoneDelegate.invokeTask (/Users/wangshuo/Projects/ksite_webapps/node_modules/zone.js/dist/zone-node.js:421:31)
    at Zone.runTask (/Users/wangshuo/Projects/ksite_webapps/node_modules/zone.js/dist/zone-node.js:188:47)
    at ZoneTask.invokeTask (/Users/wangshuo/Projects/ksite_webapps/node_modules/zone.js/dist/zone-node.js:496:34)
    at Socket.ZoneTask.invoke (/Users/wangshuo/Projects/ksite_webapps/node_modules/zone.js/dist/zone-node.js:485:48)
    at emitNone (events.js:106:13)
    at Socket.emit (events.js:208:7)
    at emitReadable_ (_stream_readable.js:513:10)
    at emitReadable (_stream_readable.js:507:7)
    at addChunk (_stream_readable.js:274:7)
    at readableAddChunk (_stream_readable.js:250:11)
    at Socket.Readable.push (_stream_readable.js:208:10)
    at TCP.onread (net.js:597:20)

Pipeline Compat

Was think a little about interoperability with pipeline and how that would work in user friendly and performant way.

Was think something like this would maybe be nice?

await pipeline(
  src,
  client.pipeline(
    options, 
    async function* req() {
      yield stuff
    }, 
    async function* res(statusCode, headers, body) {
      yield* body
    }
  ),
  dst
)

This could be implemented in user land as well using a helper method. But there might be some considerations we could do to make this possible in a performance way. Other than that it might or might not make sense to include such a helper in this library to encourage usage?

First of all adding support for request body to be a generator function. The user could of course just use Readable.from, but native support would make things faster.

The next question becomes out the response side. Might or might not be possible to implement the response as a native generator, but it would be difficult to support both streams and generators.

should we validate the headers?

I noticed a todo on the request headers in regards to whether or not to validate them.

I see undici more like a low level library which implements the protocol in an efficient manner. If the user provides invalid headers the server should simply send an error response? I think validation should live in higher level libraries that wrap undici, e.g. got.

Thoughts?

return null response when content-length 0?

We currently return null as response body when it's a HEAD request.

We could also do the same when the response contains a content-length: 0 header?

Alternatively, always return a Readable for body to keep the response signature consistent.

Also, I'm a little unsure what happens with HEAD + skipBody if the server actually incorrectly sends a body?

pipelining should be relative number active requests?

Currently the logic is only relative to number of queued/inflight (?) requests, however a request might still be receiving the body before the next one is dispatched, e.g. with pipelining 1 and a GET request with a large body, the next request would be dispatched before the body has been fully received.

Is this intentional or a bug?

reset parser on disconnect?

I think we need to reset/recreate the parser on socket failure/disconnect? I don' t believe this is the case currently?

socket 'finish' and/or 'error' should error in-flight requests

If the socket emits 'finish' or 'end' while a request is in-flight it should call socket.destroy(new Error('aborted')) in order to properly error in-flight requests.

Would require #19.

eos only handles the case of premature close. However, a net.Socket can emit 'end' and or 'finish' even though the request has failed.

var vs let?

I noticed this library is using var instead of let/const in some places. Is this intentional and how future code should be written as well?

Dynamically placing requests to connections queues and pipelines

Right now this project works by statically creating a pool of connection (say 100), and then for each of those collections there is a queue. From that queue, n concurrent requests are pipelined (10) together to upstream.

The allocation of those requests to connections and their queues is currently round-robin. However, the optimal solution requires to always have each client at their full pipeline, and a request always ready to go, so that the TCP window of the connection stays wide open.

The problem is that one failing connection can bring down the whole pipeline, and it's quite a catastrophic event, because one failure can have n requests failing.
Also, slow requests have a huge impact on the pipeline size, as 1 requests that takes 500ms to reply will delay all the subsequent requests of 500ms.

Tuning the number of concurrent connections can swing the throughput up to 100% in my tests. I think we should find a nice algorithm to place the requests.

@lucamaraschi @delvedor do you have any idea to propose from your AI studies? This looks like a complex Knapsack problem

Disabling pipelining

As far as I can see, right now it is impossible to use this library in a way that only a single request is on the wire at the time.

Would it make sense to add pipeling: 0 support which enforces that requests are first sent and received before the next one is sent.

backpressure algorithm flaky test

 ✖ should not be equal

  test/pool.js                                    
  106 |                                           
  107 |   t.strictEqual(d3.client, d2.client)     
> 108 |   t.notStrictEqual(d3.client, d4.client)  
      | ----^                                     
  109 |                                           
  110 |   d3.client.emit('drain')                 
  111 |                                

Option to use core stream

Would it be possible and make sense to add a runtime option to use core streams instead of readable-stream and eos? In particular in terms of the response body.

Tests

meta list of things that might need more tests:

  • aborting request
  • aborting response
  • server errors
  • pipelining request bodies
  • resuming from response backpressure
  • parser errors
  • timeout handling
  • reconnect backof (use sinonjs fake-timers to speed up time)
  • leaking event handlers on socket
  • response body autoDestroy
  • request body destroy on failure, don't destroy on success (pipeline behavior)
  • callbacks always invoked in nextTick
  • what happens if content-length is less/greater than actual body?
  • async wrap all callbacks
  • all callbacks are always invoked
  • drain does not occur unexpectedly
  • destroy never leads to unhandled rejection
  • ensure callbacks always have null on err and val instead of undefined.

... adding more stuff as they come up

Server closes connection on chunked pipelining

This was a bit surprising for me. It seem the server always ends the connection at the end of a request which sent a chunked body, i.e. this will always close the connection once the first request has completed.

@mcollina Are you familiar with this behavior?

test('pipelined POST does not interleave', (t) => {
  t.plan(5)

  const server = createServer((req, res) => {
    req.on('data', chunk => {
      console.error(chunk)
    }).on('end', () => {
      res.end()
    })
  })
  t.tearDown(server.close.bind(server))

  server.listen(0, () => {
    const client = new Client(`http://localhost:${server.address().port}`, {
      pipelining: 2
    })

    let a = 0
    client.request({
      path: '/',
      method: 'POST',
      body: new Readable({
        read () {
          this.push(a++ > 128 ? null : 'a')
        }
      })
    }, (err, data) => {
      console.error('a', err)
    })

    let b = 0
    client.request({
      path: '/',
      method: 'POST',
      body: new Readable({
        read () {
          this.push(b++ > 128 ? null : 'b')
        }
      })
    }, (err, data) => {
      console.error('b', err)
      // t.ok(err)
    })
  })
})

Perpetually attempts reconnecting on socket error

Updated Issue Description

Okay, I see what is happening. So the client reconnects on the emission of error from socket. It also does so without end. I presume this is by design.

I'm sure there are plenty of cases where that is needed. However, there are also others where it is problematic.

I've been working on replacing http-proxy for a custom reverse proxy I manage. Among the things that proxy does is use proxy errors to signal potential problems with the backend services it manages, as well as do some automated recovery in certain cases.

That being the case having visibility of the inability to reach said services is crucial to the management and recovery of them. It may be that this module is not well suited for that use case, but that is the use case I'm attempting with it atm.

I would propose a solution, but I'm honestly not sure what the best solution would be. I'm going to hack on things locally and see if anything worthwhile comes to mind.

But please let me know your thoughts as I would be happy to do a PR if you have any ideas to manage this without breaking the design philosophy of the module.

Original Issue Description

Request Hangs on No Response

Repro is straight forward. With nothing listening on port 3000 run the following and nothing will happen. The timeout setting has no effect either.

const { Client } = require('undici')
const client = new Client(`http://localhost:3000`)

client.request({ path: '/', method: 'GET' }, (err, data) => console.log(err || data || 'nothing'))

Limit reading of aborted response body

i.e.

request(options, (err, { body }) => {
  body.destroy(new Error('aborted'))
})

It looks to me like the request keeps processing? Not sure how this can be aborted but maybe it should kill and reconnect the socket?

Does backpressure on response body work?

Reading through the code I can't see that backpressure is applied from the response body.

i.e.

      this.parser[HTTPParser.kOnBody] = (chunk, offset, length) => {
        this[kLastBody].push(chunk.slice(offset, offset + length))
      }

Does nothing with the return value of push which should be indicating whether more data should be received or not...

auto retry inflight pipeline requests?

So if we send e.g. 3 pipelined requests to the server, the first one fails e.g. due to socket failure, then we should probably fail the first one, reconnect and automatically retry the two following ones since the server never processed them?

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.