nodejs / nodejs/undici

Support interoperability with other version of itself

Open
#3,071 8 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Docs enhancement
Dominant language
JavaScript
Stars
7.7k
Forks
880
Avg merge
2d 16h
Merged PRs (30d)
68

Description

This would solve...

undici is not compatible with other versions of itself, or Node's.

If a library uses undici's fetch() internally and the user of the library uses Node's Request, or vice versa, fetch() call will fail with an error like so:

  TypeError: Failed to parse URL from [object Request]
      at fetch (~/my-lib/node_modules/.pnpm/undici@6.11.1/node_modules/undici/index.js:109:13) {
    [cause]: TypeError: Invalid URL
        at new URL (node:internal/url:804:36)
        at new Request (~/my-lib/node_modules/.pnpm/undici@6.11.1/node_modules/undici/lib/web/fetch/request.js:88:21)
        at fetch (~/my-lib/node_modules/.pnpm/undici@6.11.1/node_modules/undici/lib/web/fetch/index.js:136:21)
        at fetch (~/my-lib/node_modules/.pnpm/undici@6.11.1/node_modules/undici/index.js:106:18)
        at fetch (~/my-lib/packages/pds/dist/auth-provider.js:42:51)
        at fetchTimeout (~/my-lib/packages/fetch/dist/fetch-wrap.js:61:28)
        at ~/my-lib/packages/fetch/dist/fetch-wrap.js:39:29
        at ~/my-lib/packages/transformer/dist/compose.js:20:47
        at async i (~/my-lib/packages/transformer/dist/compose.js:16:51)
        at async CachedGetter.get (~/my-lib/packages/caching/dist/cached-getter.js:97:20) {
      code: 'ERR_INVALID_URL',
      input: '[object Request]'
    }

This error is misleading as the input is actually a Request that gets casted to string because it comes from another version of undici:

https://github.com/nodejs/undici/blob/f51f226522ec75a0613a07a4efc8f78938030c45/lib/web/fetch/request.js#L924-L934

The implementation should look like...

When checking if an input is a request, instead of using instanceof, an thorough interface check could be performed.
Every fields from the input should be checked against the spec's interface https://fetch.spec.whatwg.org/#requestinfo

webidl.converters.RequestInfo = function (V) {
  if (typeof V === 'string') {
    return webidl.converters.USVString(V)
  }

  if (V instanceof Request) {
    return webidl.converters.Request(V)
  }

  const str = webidl.converters.USVString(V)
  if (str === '[object Request]') {
    // Request from another version of undici (or node's internal undici)
    return webidl.converters.RequestInterface(V) // <== TODO implement this
  }

  return src
}

Note that every field of the input Request (including dispatcher) should be carried over to the new request.

Symbols would probably need to be using a globally namespaced form instead of locally declared symbols (Symbol.for('undici.XYZ')) for the initalization process to work here:

https://github.com/nodejs/undici/blob/f51f226522ec75a0613a07a4efc8f78938030c45/lib/web/fetch/request.js#L107-L118

I have also considered...

Additional context

I am trying to make a library to ease the use of fetch() by allowing to tranform Requests, and wrap node's fetch(). This is not possible if Request/fetch from different sources are mixed together

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/web/fetch/request.js at the RequestInfo conversion and request initialization areas referenced by the issue. Read the Fetch RequestInfo specification and trace how cross-version Request objects and symbols are recognized. Done means requests from other undici versions or Node's implementation work with fetch while preserving all input fields, including dispatcher.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
backend, networking
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.