Skip to content

Revert "fix: patch undici File support on Node.js < 20 (#588)" #592

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.

Sign up for GitHub

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

Closed
fengmk2 wants to merge 1 commit into from
Closed

Revert "fix: patch undici File support on Node.js < 20 (#588)" #592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/HttpClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ export class HttpClient extends EventEmitter {
const requestOptions: IUndiciRequestOption = {
method,
// disable undici auto redirect handler
// maxRedirections: 0,
maxRedirections: 0,

@coderabbitai coderabbitai Bot Aug 12, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

TS2353: maxRedirections is not in undici typings across versions

CI fails with TS2353 because some undici typings don’t include maxRedirections. We still want to disable undici’s built-in redirects when supported. Assign the property post-creation with a type escape to satisfy all supported undici versions.

Apply this diff:

       const requestOptions: IUndiciRequestOption = {
         method,
-        // disable undici auto redirect handler
-        maxRedirections: 0,
         headersTimeout,
         headers,
         bodyTimeout,
         opaque: internalOpaque,
         dispatcher: args.dispatcher ?? this.#dispatcher,
         signal: args.signal,
         reset: false,
       };
+      // Disable undici auto redirect handling when supported by the runtime undici version.
+      // Some undici typings (e.g., v7) don’t declare `maxRedirections`, so set it via a type-escape.
+      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+      // @ts-expect-error: 'maxRedirections' is not declared in some undici versions' types
+      (requestOptions as any).maxRedirections = 0;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
maxRedirections: 0,
const requestOptions: IUndiciRequestOption = {
method,
headersTimeout,
headers,
bodyTimeout,
opaque: internalOpaque,
dispatcher: args.dispatcher ?? this.#dispatcher,
signal: args.signal,
reset: false,
};
// Disable undici auto redirect handling when supported by the runtime undici version.
// Some undici typings (e.g., v7) don’t declare `maxRedirections`, so set it via a type-escape.
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error: 'maxRedirections' is not declared in some undici versions' types
(requestOptions as any).maxRedirections = 0;
🧰 Tools
🪛 GitHub Actions: Node.js 16 CI

[error] 432-432: TS2353: Object literal may only specify known properties, and 'maxRedirections' does not exist in type 'IUndiciRequestOption'.

🪛 GitHub Actions: Publish Any Commit

[error] 432-432: TypeScript error TS2353 during npm run build: Object literal may only specify known properties, and 'maxRedirections' does not exist in type 'IUndiciRequestOption'.

🤖 Prompt for AI Agents 
In src/HttpClient.ts around line 432, the undici option maxRedirections is
causing TS2353 because some undici typings don’t include it; remove it from the
initial options and instead, immediately after creating the undici client,
assign the property on the client instance using a type escape (cast the client
to any/unknown and set .maxRedirections = 0) so the runtime disables built-in
redirects when supported while satisfying all TypeScript versions.

headersTimeout,
headers,
bodyTimeout,
Expand Down
1 change: 0 additions & 1 deletion src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { LRU } from 'ylru';
import { patchForNode16 } from './utils.js';


patchForNode16();

import { HttpClient, HEADER_USER_AGENT } from './HttpClient.js';
Expand Down
34 changes: 1 addition & 33 deletions src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import { randomBytes, createHash } from 'node:crypto';
import { Readable } from 'node:stream';
import { performance } from 'node:perf_hooks';
import { ReadableStream, TransformStream } from 'node:stream/web';
import { Blob, File } from 'node:buffer';
import { toUSVString } from 'node:util';
import { Blob } from 'node:buffer';
import type { FixJSONCtlChars } from './Request.js';
import { SocketInfo } from './Response.js';
import symbols from './symbols.js';
Expand Down Expand Up @@ -232,37 +231,6 @@ export function patchForNode16() {
// @ts-ignore
global.DOMException = getDOMExceptionClass();
}
// multi undici version in node version less than 20 https://github.com/nodejs/undici/issues/4374
if (typeof global.File === 'undefined') {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
global.File = File;
}

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
if (String.prototype.toWellFormed === undefined) {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
String.prototype.toWellFormed = function() {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
return toUSVString(this);
};
}

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
if (String.prototype.isWellFormed === undefined) {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
String.prototype.isWellFormed = function() {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
return toUSVString(this) === this;
};
}

}

// https://github.com/jimmywarting/node-domexception/blob/main/index.js
Expand Down
5 changes: 1 addition & 4 deletions test/HttpClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,8 @@ import { describe, it, beforeAll, afterAll } from 'vitest';
import selfsigned from 'selfsigned';
import { HttpClient, RawResponseWithMeta, getGlobalDispatcher } from '../src/index.js';
import { startServer } from './fixtures/server.js';
import { nodeMajorVersion } from './utils.js';

const pems = selfsigned.generate([], {
keySize: nodeMajorVersion() >= 22 ? 2048 : 1024,
});
const pems = selfsigned.generate();

if (process.env.ENABLE_PERF) {
const obs = new PerformanceObserver(items => {
Expand Down
5 changes: 1 addition & 4 deletions test/diagnostics_channel.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import type {
} from '../src/index.js';
import symbols from '../src/symbols.js';
import { startServer } from './fixtures/server.js';
import { nodeMajorVersion } from './utils.js';

describe('diagnostics_channel.test.ts', () => {
let close: any;
Expand Down Expand Up @@ -144,9 +143,7 @@ describe('diagnostics_channel.test.ts', () => {
});

it('should support trace socket info with H2 by undici:client:sendHeaders and undici:request:trailers', async () => {
const pem = selfsigned.generate([], {
keySize: nodeMajorVersion() >= 22 ? 2048 : 1024,
});
const pem = selfsigned.generate();
const server = createSecureServer({
key: pem.private,
cert: pem.cert,
Expand Down
6 changes: 2 additions & 4 deletions test/fixtures/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import busboy from 'busboy';
import iconv from 'iconv-lite';
import selfsigned from 'selfsigned';
import qs from 'qs';
import { nodeMajorVersion, readableToBytes } from '../utils.js';
import { readableToBytes } from '../utils.js';

const requestsPerSocket = Symbol('requestsPerSocket');

Expand Down Expand Up @@ -370,9 +370,7 @@ export async function startServer(options?: {
};

if (options?.https) {
const pem = selfsigned.generate([], {
keySize: nodeMajorVersion() >= 22 ? 2048 : 1024,
});
const pem = selfsigned.generate();
server = createHttpsServer({
key: pem.private,
cert: pem.cert,
Expand Down
2 changes: 1 addition & 1 deletion test/options.stream.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('options.stream.test.ts', () => {
assert.equal(response.headers['content-type'], 'application/json');
assert.equal(response.data.method, 'POST');
// console.log(response.data);
// assert.match(response.data.headers['content-type'], /^multipart\/form-data; boundary=--------------------------\d+$/);
assert.match(response.data.headers['content-type'], /^multipart\/form-data; boundary=--------------------------\d+$/);

@coderabbitai coderabbitai Bot Aug 12, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix failing boundary assertion (boundary includes hex chars, not only digits)

CI shows the boundary contains a hex string (0-9a-f), so the digits-only regex is too strict.

Apply this diff to make the assertion robust:

-    assert.match(response.data.headers['content-type'], /^multipart\/form-data; boundary=--------------------------\d+$/);
+    // boundary is a crypto-ish hex string; allow hex chars instead of digits only
+    assert.match(response.data.headers['content-type'], /^multipart\/form-data; boundary=--------------------------[0-9a-f]+$/i);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
assert.match(response.data.headers['content-type'], /^multipart\/form-data; boundary=--------------------------\d+$/);
// boundary is a crypto-ish hex string; allow hex chars instead of digits only
assert.match(
response.data.headers['content-type'],
/^multipart\/form-data; boundary=--------------------------[0-9a-f]+$/i
);
🧰 Tools
🪛 GitHub Check: Node.js / Test (macos-latest, 22)

[failure] 97-97: test/options.stream.test.ts > options.stream.test.ts > should upload file with formstream
AssertionError: The input did not match the regular expression /^multipart/form-data; boundary=--------------------------\d+$/. Input:

'multipart/form-data; boundary=--------------------------86442430eab6492fe5b8831b'

  • Expected:
    /^multipart/form-data; boundary=--------------------------\d+$/
  • Received:
    "multipart/form-data; boundary=--------------------------86442430eab6492fe5b8831b"

❯ test/options.stream.test.ts:97:12

🪛 GitHub Check: Node.js / Test (ubuntu-latest, 24)

[failure] 97-97: test/options.stream.test.ts > options.stream.test.ts > should upload file with formstream
AssertionError: The input did not match the regular expression /^multipart/form-data; boundary=--------------------------\d+$/. Input:

'multipart/form-data; boundary=--------------------------2fdf9d79bde30b3b3cd5ae8d'

  • Expected:
    /^multipart/form-data; boundary=--------------------------\d+$/
  • Received:
    "multipart/form-data; boundary=--------------------------2fdf9d79bde30b3b3cd5ae8d"

❯ test/options.stream.test.ts:97:12

🪛 GitHub Check: Node.js / Test (macos-latest, 24)

[failure] 97-97: test/options.stream.test.ts > options.stream.test.ts > should upload file with formstream
AssertionError: The input did not match the regular expression /^multipart/form-data; boundary=--------------------------\d+$/. Input:

'multipart/form-data; boundary=--------------------------c32713078e5e011b81e48fe6'

  • Expected:
    /^multipart/form-data; boundary=--------------------------\d+$/
  • Received:
    "multipart/form-data; boundary=--------------------------c32713078e5e011b81e48fe6"

❯ test/options.stream.test.ts:97:12

🪛 GitHub Check: Node.js / Test (macos-latest, 20)

[failure] 97-97: test/options.stream.test.ts > options.stream.test.ts > should upload file with formstream
AssertionError: The input did not match the regular expression /^multipart/form-data; boundary=--------------------------\d+$/. Input:

'multipart/form-data; boundary=--------------------------c8cec534105c72d9cff8a19a'

  • Expected:
    /^multipart/form-data; boundary=--------------------------\d+$/
  • Received:
    "multipart/form-data; boundary=--------------------------c8cec534105c72d9cff8a19a"

❯ test/options.stream.test.ts:97:12

🪛 GitHub Check: Node.js / Test (ubuntu-latest, 20)

[failure] 97-97: test/options.stream.test.ts > options.stream.test.ts > should upload file with formstream
AssertionError: The input did not match the regular expression /^multipart/form-data; boundary=--------------------------\d+$/. Input:

'multipart/form-data; boundary=--------------------------aad8037a0afa31756ffbeaaf'

  • Expected:
    /^multipart/form-data; boundary=--------------------------\d+$/
  • Received:
    "multipart/form-data; boundary=--------------------------aad8037a0afa31756ffbeaaf"

❯ test/options.stream.test.ts:97:12

🪛 GitHub Check: Node.js / Test (ubuntu-latest, 22)

[failure] 97-97: test/options.stream.test.ts > options.stream.test.ts > should upload file with formstream
AssertionError: The input did not match the regular expression /^multipart/form-data; boundary=--------------------------\d+$/. Input:

'multipart/form-data; boundary=--------------------------8c01933016ec59398b3dc4ed'

  • Expected:
    /^multipart/form-data; boundary=--------------------------\d+$/
  • Received:
    "multipart/form-data; boundary=--------------------------8c01933016ec59398b3dc4ed"

❯ test/options.stream.test.ts:97:12

🤖 Prompt for AI Agents 
In test/options.stream.test.ts around line 97, the current assertion expects the
multipart boundary to be only digits which is incorrect because CI shows the
boundary includes hex characters; update the regex to allow hexadecimal
characters (e.g. use [0-9a-fA-F]+ or add the /i flag) so the assertion matches
boundaries containing letters a–f as well as digits.

assert.equal(response.data.files.file.filename, 'options.stream.test.ts');
assert.equal(response.data.form.hello, '你好 urllib 3');
const raw = await readFile(__filename);
Expand Down
5 changes: 1 addition & 4 deletions test/options.timeout.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,8 @@ import selfsigned from 'selfsigned';
import { describe, it, beforeAll, afterAll } from 'vitest';
import urllib, { HttpClientRequestTimeoutError, HttpClient } from '../src/index.js';
import { startServer } from './fixtures/server.js';
import { nodeMajorVersion } from './utils.js';

const pems = selfsigned.generate([], {
keySize: nodeMajorVersion() >= 22 ? 2048 : 1024,
});
const pems = selfsigned.generate();

describe('options.timeout.test.ts', () => {
let close: any;
Expand Down
5 changes: 1 addition & 4 deletions test/urllib.options.rejectUnauthorized-false.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { describe, it, beforeAll, afterAll } from 'vitest';
import selfsigned from 'selfsigned';
import urllib, { HttpClient } from '../src/index.js';
import { startServer } from './fixtures/server.js';
import { nodeMajorVersion } from './utils.js';

describe('urllib.options.rejectUnauthorized-false.test.ts', () => {
let close: any;
Expand All @@ -30,9 +29,7 @@ describe('urllib.options.rejectUnauthorized-false.test.ts', () => {
});

it('should 200 with H2 on options.rejectUnauthorized = false', async () => {
const pem = selfsigned.generate([], {
keySize: nodeMajorVersion() >= 22 ? 2048 : 1024,
});
const pem = selfsigned.generate();
const server = createSecureServer({
key: pem.private,
cert: pem.cert,
Expand Down