From a8c5aafb21b19f73e45e5ac21034d40d5bd50002 Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Thu, 28 Nov 2024 12:02:02 +0800 Subject: [PATCH 01/10] feat: upgrade to undici v7 fix server side close unexpected exit closes https://github.com/node-modules/urllib/issues/541 --- .../h2-other-side-closed-exit-0-fetch.cjs | 34 +++++++++++++ examples/h2-other-side-closed-exit-0.cjs | 34 +++++++++++++ examples/longruning.cjs | 49 +++++++++++++++++++ package.json | 2 +- src/FetchOpaqueInterceptor.ts | 2 +- src/HttpAgent.ts | 2 +- src/HttpClient.ts | 10 +++- src/fetch.ts | 13 +++-- 8 files changed, 134 insertions(+), 12 deletions(-) create mode 100644 examples/h2-other-side-closed-exit-0-fetch.cjs create mode 100644 examples/h2-other-side-closed-exit-0.cjs create mode 100644 examples/longruning.cjs diff --git a/examples/h2-other-side-closed-exit-0-fetch.cjs b/examples/h2-other-side-closed-exit-0-fetch.cjs new file mode 100644 index 00000000..00b3b6b5 --- /dev/null +++ b/examples/h2-other-side-closed-exit-0-fetch.cjs @@ -0,0 +1,34 @@ +const { fetch, setGlobalDispatcher, Agent } = require('..'); + +setGlobalDispatcher(new Agent({ + allowH2: true, +})); + +async function main() { + for (let i = 0; i < 100; i++) { + try { + const r = await fetch('https://edgeupdates.microsoft.com/api/products'); + console.log(r.status, r.headers, (await r.text()).length); + } catch (err) { + // console.error(err); + // throw err; + if (err.code === 'UND_ERR_SOCKET') { + continue; + } else { + throw err; + } + } + } +} + +main().then(() => { + console.log('main end'); +}).catch(err => { + console.error('main error throw: %s', err); + // console.error(err); + process.exit(1); +}); + +process.on('beforeExit', (...args) => { + console.error('beforeExit', args); +}); diff --git a/examples/h2-other-side-closed-exit-0.cjs b/examples/h2-other-side-closed-exit-0.cjs new file mode 100644 index 00000000..a278ffe2 --- /dev/null +++ b/examples/h2-other-side-closed-exit-0.cjs @@ -0,0 +1,34 @@ +const { request, Agent, setGlobalDispatcher } = require('undici'); + +setGlobalDispatcher(new Agent({ + allowH2: true, +})); + +async function main() { + for (let i = 0; i < 100; i++) { + try { + const r = await request('https://edgeupdates.microsoft.com/api/products'); + console.log(r.statusCode, r.headers, (await r.body.blob()).size); + } catch (err) { + // console.error(err); + // throw err; + if (err.code === 'UND_ERR_SOCKET') { + continue; + } else { + throw err; + } + } + } +} + +main().then(() => { + console.log('main end'); +}).catch(err => { + console.error('main error throw: %s', err); + // console.error(err); + process.exit(1); +}); + +process.on('beforeExit', (...args) => { + console.error('beforeExit', args); +}); diff --git a/examples/longruning.cjs b/examples/longruning.cjs new file mode 100644 index 00000000..1db2b8ce --- /dev/null +++ b/examples/longruning.cjs @@ -0,0 +1,49 @@ +const { HttpClient } = require('..'); + +const httpClient = new HttpClient({ + allowH2: true, +}); + +async function main() { + for (let i = 0; i < 1000000; i++) { + // await httpClient.request('https://registry.npmmirror.com/'); + // console.log(r.status, r.headers, r.res.timing); + try { + const r = await httpClient.request('https://edgeupdates.microsoft.com/api/products'); + // console.log(r.status, r.headers, r.data.length, r.res.timing); + if (i % 10 === 0) { + // console.log(r.status, r.headers, r.data.length, r.res.timing); + console.log(i, r.status, process.memoryUsage()); + } + } catch (err) { + console.error('%s error: %s', i, err.message); + } + } +} + +main().then(() => { + console.log('main end'); +}).catch(err => { + console.error('main error throw: %s', err); + console.error(err); + process.exit(1); +}); + +// process.on('uncaughtException', (...args) => { +// console.error('uncaughtException', args); +// process.exit(1); +// }); + +// process.on('unhandledRejection', (...args) => { +// console.error('unhandledRejection', args); +// process.exit(2); +// }); + +// process.on('uncaughtExceptionMonitor', (...args) => { +// console.error('uncaughtExceptionMonitor', args); +// process.exit(2); +// }); + +process.on('beforeExit', (...args) => { + console.error('beforeExit', args); +}); diff --git a/package.json b/package.json index 5e330e01..c333d248 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "mime-types": "^2.1.35", "qs": "^6.12.1", "type-fest": "^4.20.1", - "undici": "^6.19.2", + "undici": "^7.0.0", "ylru": "^2.0.0" }, "devDependencies": { diff --git a/src/FetchOpaqueInterceptor.ts b/src/FetchOpaqueInterceptor.ts index 2a8564cd..dd741df5 100644 --- a/src/FetchOpaqueInterceptor.ts +++ b/src/FetchOpaqueInterceptor.ts @@ -32,7 +32,7 @@ export interface OpaqueInterceptorOptions { export function fetchOpaqueInterceptor(opts: OpaqueInterceptorOptions) { const opaqueLocalStorage = opts?.opaqueLocalStorage; return (dispatch: Dispatcher['dispatch']): Dispatcher['dispatch'] => { - return function redirectInterceptor(opts: Dispatcher.DispatchOptions, handler: Dispatcher.DispatchHandlers) { + return function redirectInterceptor(opts: Dispatcher.DispatchOptions, handler: Dispatcher.DispatchHandler) { const opaque = opaqueLocalStorage?.getStore(); (handler as any).opaque = opaque; return dispatch(opts, handler); diff --git a/src/HttpAgent.ts b/src/HttpAgent.ts index 5b38121d..cb1200bf 100644 --- a/src/HttpAgent.ts +++ b/src/HttpAgent.ts @@ -70,7 +70,7 @@ export class HttpAgent extends Agent { this.#checkAddress = options.checkAddress; } - dispatch(options: Agent.DispatchOptions, handler: Dispatcher.DispatchHandlers): boolean { + dispatch(options: Agent.DispatchOptions, handler: Dispatcher.DispatchHandler): boolean { if (this.#checkAddress && options.origin) { const originUrl = typeof options.origin === 'string' ? new URL(options.origin) : options.origin; let hostname = originUrl.hostname; diff --git a/src/HttpClient.ts b/src/HttpClient.ts index f05d5b52..af4d0969 100644 --- a/src/HttpClient.ts +++ b/src/HttpClient.ts @@ -49,7 +49,7 @@ type IUndiciRequestOption = PropertyShouldBe 0 && requestContext.socketErrorRetries < args.socketErrorRetry) { requestContext.socketErrorRetries++; + debug('Request#%d retry on socket error, socketErrorRetries: %d', + requestId, requestContext.socketErrorRetries); return await this.#requestInternal(url, options, requestContext); } } diff --git a/src/fetch.ts b/src/fetch.ts index d7c197c3..5c51b083 100644 --- a/src/fetch.ts +++ b/src/fetch.ts @@ -8,21 +8,20 @@ import { Agent, getGlobalDispatcher, Pool, - Dispatcher, } from 'undici'; // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore import undiciSymbols from 'undici/lib/core/symbols.js'; // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore -import undiciFetchSymbols from 'undici/lib/web/fetch/symbols.js'; +import { getResponseState } from 'undici/lib/web/fetch/response.js'; import { channels, ClientOptions, PoolStat, RequestDiagnosticsMessage, ResponseDiagnosticsMessage, - UnidiciTimingInfo, + UndiciTimingInfo, } from './HttpClient.js'; import { HttpAgent, @@ -51,7 +50,7 @@ export type FetchDiagnosticsMessage = { export type FetchResponseDiagnosticsMessage = { fetch: FetchMeta; - timingInfo?: UnidiciTimingInfo; + timingInfo?: UndiciTimingInfo; response?: Response; error?: Error; }; @@ -236,8 +235,8 @@ export class FetchFactory { throw e; } - // get unidici internal response - const state = Reflect.get(res!, undiciFetchSymbols.kState) as Dispatcher.ResponseData; + // get undici internal response + const state = getResponseState(res!); updateSocketInfo(socketInfo, internalOpaque /* , rawError */); urllibResponse.headers = convertHeader(res!.headers); @@ -250,7 +249,7 @@ export class FetchFactory { channels.fetchResponse.publish({ fetch: fetchMeta, - timingInfo: (state as any).timingInfo, + timingInfo: state.timingInfo, response: res!, } as FetchResponseDiagnosticsMessage); channels.response.publish({ From b171bc3c55ffeb448ed20e8798e7fd40828f6c20 Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Thu, 28 Nov 2024 21:49:31 +0800 Subject: [PATCH 02/10] use form-data to upload file --- package.json | 2 + src/FormData.ts | 32 ++++++++++++++ src/HttpClient.ts | 54 ++++++++--------------- src/diagnosticsChannel.ts | 16 +++++-- test/HttpClient.test.ts | 67 ++++++++++++++++++++++++++--- test/diagnostics_channel.test.ts | 3 +- test/fetch.test.ts | 6 ++- test/options.dispatcher.test.ts | 2 +- test/options.files.test.ts | 14 +++--- test/options.followRedirect.test.ts | 10 ++--- 10 files changed, 149 insertions(+), 57 deletions(-) create mode 100644 src/FormData.ts diff --git a/package.json b/package.json index c333d248..f04963d2 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ "prepublishOnly": "npm run build" }, "dependencies": { + "form-data": "^4.0.1", "formstream": "^1.5.1", "mime-types": "^2.1.35", "qs": "^6.12.1", @@ -68,6 +69,7 @@ "cross-env": "^7.0.3", "eslint": "8", "eslint-config-egg": "14", + "https-pem": "^3.0.0", "iconv-lite": "^0.6.3", "proxy": "^1.0.2", "selfsigned": "^2.0.1", diff --git a/src/FormData.ts b/src/FormData.ts new file mode 100644 index 00000000..78b54a8f --- /dev/null +++ b/src/FormData.ts @@ -0,0 +1,32 @@ +import path from 'node:path'; +import _FormData from 'form-data'; + +export class FormData extends _FormData { + _getContentDisposition(value: any, options: any) { + // support non-ascii filename + // https://github.com/form-data/form-data/pull/571 + let filename; + let contentDisposition; + + if (typeof options.filepath === 'string') { + // custom filepath for relative paths + filename = path.normalize(options.filepath).replace(/\\/g, '/'); + } else if (options.filename || value.name || value.path) { + // custom filename take precedence + // formidable and the browser add a name property + // fs- and request- streams have path property + filename = path.basename(options.filename || value.name || value.path); + } else if (value.readable && value.hasOwnProperty('httpVersion')) { + // or try http response + filename = path.basename(value.client._httpMessage.path || ''); + } + + if (filename) { + // https://datatracker.ietf.org/doc/html/rfc6266#section-4.1 + // support non-ascii filename + contentDisposition = 'filename="' + filename + '"; filename*=UTF-8\'\'' + encodeURIComponent(filename); + } + + return contentDisposition; + } +} diff --git a/src/HttpClient.ts b/src/HttpClient.ts index af4d0969..6d7c3fb8 100644 --- a/src/HttpClient.ts +++ b/src/HttpClient.ts @@ -9,7 +9,6 @@ import { gunzipSync, brotliDecompressSync, } from 'node:zlib'; -import { Blob } from 'node:buffer'; import { Readable, pipeline } from 'node:stream'; import { pipeline as pipelinePromise } from 'node:stream/promises'; import { basename } from 'node:path'; @@ -19,13 +18,13 @@ import { performance } from 'node:perf_hooks'; import querystring from 'node:querystring'; import { setTimeout as sleep } from 'node:timers/promises'; import { - FormData, request as undiciRequest, Dispatcher, Agent, getGlobalDispatcher, Pool, } from 'undici'; +import { FormData } from './FormData.js'; // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore import undiciSymbols from 'undici/lib/core/symbols.js'; @@ -116,28 +115,6 @@ export type ClientOptions = { }, }; -// https://github.com/octet-stream/form-data -class BlobFromStream { - #stream; - #type; - constructor(stream: Readable, type: string) { - this.#stream = stream; - this.#type = type; - } - - stream() { - return this.#stream; - } - - get type(): string { - return this.#type; - } - - get [Symbol.toStringTag]() { - return 'Blob'; - } -} - export const VERSION = 'VERSION'; // 'node-urllib/4.0.0 Node.js/18.19.0 (darwin; x64)' export const HEADER_USER_AGENT = @@ -490,21 +467,28 @@ export class HttpClient extends EventEmitter { } } for (const [ index, [ field, file, customFileName ]] of uploadFiles.entries()) { + let fileName = ''; + let value: any; if (typeof file === 'string') { - // FIXME: support non-ascii filename - // const fileName = encodeURIComponent(basename(file)); - // formData.append(field, await fileFromPath(file, `utf-8''${fileName}`, { type: mime.lookup(fileName) || '' })); - const fileName = basename(file); - const fileReadable = createReadStream(file); - formData.append(field, new BlobFromStream(fileReadable, mime.lookup(fileName) || ''), fileName); + fileName = basename(file); + value = createReadStream(file); } else if (Buffer.isBuffer(file)) { - formData.append(field, new Blob([ file ]), customFileName || `bufferfile${index}`); + fileName = customFileName || `bufferfile${index}`; + value = file; } else if (file instanceof Readable || isReadable(file as any)) { - const fileName = getFileName(file) || customFileName || `streamfile${index}`; - formData.append(field, new BlobFromStream(file, mime.lookup(fileName) || ''), fileName); + fileName = getFileName(file) || customFileName || `streamfile${index}`; isStreamingRequest = true; + value = file; } + const mimeType = mime.lookup(fileName) || ''; + formData.append(field, value, { + filename: fileName, + contentType: mimeType, + }); + debug('formData append field: %s, mimeType: %s, fileName: %s', + field, mimeType, fileName); } + Object.assign(headers, formData.getHeaders()); requestOptions.body = formData; } else if (args.content) { if (!isGETOrHEAD) { @@ -561,8 +545,8 @@ export class HttpClient extends EventEmitter { args.socketErrorRetry = 0; } - debug('Request#%d %s %s, headers: %j, headersTimeout: %s, bodyTimeout: %s, isStreamingRequest: %s', - requestId, requestOptions.method, requestUrl.href, headers, headersTimeout, bodyTimeout, isStreamingRequest); + debug('Request#%d %s %s, headers: %j, headersTimeout: %s, bodyTimeout: %s, isStreamingRequest: %s, maxRedirections: %s', + requestId, requestOptions.method, requestUrl.href, headers, headersTimeout, bodyTimeout, isStreamingRequest, requestOptions.maxRedirections); requestOptions.headers = headers; channels.request.publish({ request: reqMeta, diff --git a/src/diagnosticsChannel.ts b/src/diagnosticsChannel.ts index 38b8e2a4..3092f85f 100644 --- a/src/diagnosticsChannel.ts +++ b/src/diagnosticsChannel.ts @@ -143,7 +143,10 @@ export function initDiagnosticsChannel() { subscribe('undici:client:sendHeaders', (message, name) => { const { request, socket } = message as DiagnosticsChannel.ClientSendHeadersMessage & { socket: SocketExtend }; const opaque = getRequestOpaque(request, kHandler); - if (!opaque || !opaque[symbols.kRequestId]) return; + if (!opaque || !opaque[symbols.kRequestId]) { + debug('[%s] opaque not found', name); + return; + } (socket[symbols.kHandledRequests] as number)++; // attach socket to opaque @@ -165,7 +168,10 @@ export function initDiagnosticsChannel() { subscribe('undici:request:bodySent', (message, name) => { const { request } = message as DiagnosticsChannel.RequestBodySentMessage; const opaque = getRequestOpaque(request, kHandler); - if (!opaque || !opaque[symbols.kRequestId]) return; + if (!opaque || !opaque[symbols.kRequestId]) { + debug('[%s] opaque not found', name); + return; + } debug('[%s] Request#%d send body', name, opaque[symbols.kRequestId]); if (!opaque[symbols.kEnableRequestTiming]) return; @@ -176,7 +182,10 @@ export function initDiagnosticsChannel() { subscribe('undici:request:headers', (message, name) => { const { request, response } = message as DiagnosticsChannel.RequestHeadersMessage; const opaque = getRequestOpaque(request, kHandler); - if (!opaque || !opaque[symbols.kRequestId]) return; + if (!opaque || !opaque[symbols.kRequestId]) { + debug('[%s] opaque not found', name); + return; + } // get socket from opaque const socket = opaque[symbols.kRequestSocket]; @@ -199,6 +208,7 @@ export function initDiagnosticsChannel() { const { request } = message as DiagnosticsChannel.RequestTrailersMessage; const opaque = getRequestOpaque(request, kHandler); if (!opaque || !opaque[symbols.kRequestId]) { + debug('[%s] opaque not found', name); return; } diff --git a/test/HttpClient.test.ts b/test/HttpClient.test.ts index 14422d27..b12347ac 100644 --- a/test/HttpClient.test.ts +++ b/test/HttpClient.test.ts @@ -1,8 +1,11 @@ import { strict as assert } from 'node:assert'; import dns from 'node:dns'; -import { sensitiveHeaders } from 'node:http2'; +import { once } from 'node:events'; +import { sensitiveHeaders, createSecureServer } from 'node:http2'; import { PerformanceObserver } from 'node:perf_hooks'; +import { setTimeout as sleep } from 'node:timers/promises'; import { describe, it, beforeAll, afterAll } from 'vitest'; +import pem from 'https-pem'; import { HttpClient, RawResponseWithMeta, getGlobalDispatcher } from '../src/index.js'; import { startServer } from './fixtures/server.js'; @@ -51,10 +54,10 @@ describe('HttpClient.test.ts', () => { }); let response = await httpClient.request('https://registry.npmmirror.com/urllib'); assert.equal(response.status, 200); - console.log(response.res.socket, response.res.timing); + // console.log(response.res.socket, response.res.timing); response = await httpClient1.request('https://registry.npmmirror.com/urllib'); assert.equal(response.status, 200); - console.log(response.res.socket, response.res.timing); + // console.log(response.res.socket, response.res.timing); // assert.equal(sensitiveHeaders in response.headers, true); assert.equal(response.headers['content-type'], 'application/json; charset=utf-8'); assert.notEqual(httpClient.getDispatcher(), getGlobalDispatcher()); @@ -74,7 +77,7 @@ describe('HttpClient.test.ts', () => { assert.equal(response.status, 200); // assert.equal(sensitiveHeaders in response.headers, true); assert.equal(response.headers['content-type'], 'application/json; charset=utf-8'); - console.log(response.res.socket, response.res.timing); + // console.log(response.res.socket, response.res.timing); await Promise.all([ httpClient.request('https://registry.npmmirror.com/urllib'), httpClient.request('https://registry.npmmirror.com/urllib'), @@ -109,10 +112,64 @@ describe('HttpClient.test.ts', () => { httpClient.request(_url), httpClient.request(_url), ]); - console.log(httpClient.getDispatcherPoolStats()); + // console.log(httpClient.getDispatcherPoolStats()); assert.equal(httpClient.getDispatcherPoolStats()['https://registry.npmmirror.com'].connected, 4); assert(httpClient.getDispatcherPoolStats()[_url.substring(0, _url.length - 1)].connected > 1); }); + + it('should not exit after other side closed error', async () => { + const server = createSecureServer(pem); + + let count = 0; + server.on('stream', (stream, headers) => { + count++; + if (count === 2) { + // SocketError: HTTP/2: "GOAWAY" frame received with code 0 + stream.session!.destroy(); + return; + } + assert.equal(headers[':method'], 'GET'); + stream.respond({ + 'content-type': 'text/plain; charset=utf-8', + 'x-custom-h2': 'hello', + ':status': 200, + }); + stream.end('hello h2!'); + }); + + server.listen(0); + await once(server, 'listening'); + + const httpClient = new HttpClient({ + allowH2: true, + connect: { + rejectUnauthorized: false, + }, + }); + + const url = `https://localhost:${server.address()!.port}`; + let response = await httpClient.request(url, { + dataType: 'text', + headers: { + 'x-my-header': 'foo', + }, + }); + assert.equal(response.status, 200); + assert.equal(response.headers['x-custom-h2'], 'hello'); + // console.log(response.res.socket, response.res.timing); + assert.equal(response.data, 'hello h2!'); + await sleep(200); + response = await httpClient.request(url, { + dataType: 'text', + headers: { + 'x-my-header': 'foo2', + }, + }); + assert.equal(response.status, 200); + assert.equal(response.headers['x-custom-h2'], 'hello'); + // console.log(response.res.socket, response.res.timing); + assert.equal(response.data, 'hello h2!'); + }); }); describe('clientOptions.defaultArgs', () => { diff --git a/test/diagnostics_channel.test.ts b/test/diagnostics_channel.test.ts index f3796494..79583b45 100644 --- a/test/diagnostics_channel.test.ts +++ b/test/diagnostics_channel.test.ts @@ -43,7 +43,8 @@ describe('diagnostics_channel.test.ts', () => { } } } - const opaque = request[kHandler].opts.opaque[symbols.kRequestOriginalOpaque]; + let opaque = request[kHandler].opts?.opaque ?? request[kHandler].opaque; + opaque = opaque[symbols.kRequestOriginalOpaque]; if (opaque && name === 'undici:client:sendHeaders' && socket) { socket[kRequests]++; opaque.tracer.socket = { diff --git a/test/fetch.test.ts b/test/fetch.test.ts index 71427ebb..74741412 100644 --- a/test/fetch.test.ts +++ b/test/fetch.test.ts @@ -1,9 +1,11 @@ import assert from 'node:assert/strict'; +import diagnosticsChannel from 'node:diagnostics_channel'; import { describe, it, beforeAll, afterAll } from 'vitest'; import { startServer } from './fixtures/server.js'; -import { fetch, FetchDiagnosticsMessage, FetchFactory, FetchResponseDiagnosticsMessage } from '../src/fetch.js'; +import { + fetch, FetchDiagnosticsMessage, FetchFactory, FetchResponseDiagnosticsMessage, +} from '../src/fetch.js'; import { RequestDiagnosticsMessage, ResponseDiagnosticsMessage } from '../src/HttpClient.js'; -import diagnosticsChannel from 'node:diagnostics_channel'; describe('fetch.test.ts', () => { let close: any; diff --git a/test/options.dispatcher.test.ts b/test/options.dispatcher.test.ts index ea133064..e8185e06 100644 --- a/test/options.dispatcher.test.ts +++ b/test/options.dispatcher.test.ts @@ -40,7 +40,7 @@ describe('options.dispatcher.test.ts', () => { assert.equal(response.status, 200); assert.equal(response.data, '

hello

'); - const response2 = await request('http://registry.npmmirror.com/urllib/latest', { + const response2 = await request('https://registry.npmmirror.com/urllib/latest', { dispatcher: proxyAgent, dataType: 'json', timing: true, diff --git a/test/options.files.test.ts b/test/options.files.test.ts index 60739d09..0119a644 100644 --- a/test/options.files.test.ts +++ b/test/options.files.test.ts @@ -214,13 +214,12 @@ describe('options.files.test.ts', () => { it('should upload a file with args.data success', async () => { const stat = await fs.stat(__filename); const largeFormValue = await fs.readFile(__filename, 'utf-8'); - // emoji not work on windows node.js >= 20 - // const txt = path.join(__dirname, 'fixtures', '๐Ÿ˜„foo๐Ÿ˜ญ.txt'); + const txtEmoji = path.join(__dirname, 'fixtures', '๐Ÿ˜„foo๐Ÿ˜ญ.txt'); const txt = path.join(__dirname, 'fixtures', 'foo.txt'); const txtValue = await fs.readFile(txt, 'utf-8'); const response = await urllib.request(`${_url}multipart`, { method: 'HEAD', - files: [ __filename ], + files: [ __filename, txtEmoji ], data: { hello: 'hello world๏ผŒ๐Ÿ˜„๐Ÿ˜“', // \r\n => \n, should encodeURIComponent first @@ -236,8 +235,13 @@ describe('options.files.test.ts', () => { assert.equal(response.data.files.file.filename, 'options.files.test.ts'); assert.equal(response.data.files.file.mimeType, 'video/mp2t'); assert.equal(response.data.files.file.size, stat.size); + assert.equal(response.data.files.file1.filename, '๐Ÿ˜„foo๐Ÿ˜ญ.txt'); + assert.equal(response.data.files.file1.mimeType, 'text/plain'); + assert.equal(response.data.files.file1.size, 24); assert.equal(response.data.form.hello, 'hello world๏ผŒ๐Ÿ˜„๐Ÿ˜“'); - assert.equal(JSON.stringify(decodeURIComponent(response.data.form.txtValue)), JSON.stringify(txtValue)); + assert.equal( + JSON.stringify(decodeURIComponent(response.data.form.txtValue)), + JSON.stringify(txtValue)); assert.equal(decodeURIComponent(response.data.form.txtValue), txtValue); assert.equal(decodeURIComponent(response.data.form.large), largeFormValue); }); @@ -293,7 +297,7 @@ describe('options.files.test.ts', () => { assert.equal(response.data.files['readable.js'].mimeType, 'application/javascript'); assert.equal(response.data.files['buffer.js'].filename, 'buffer.js'); - assert.equal(response.data.files['buffer.js'].mimeType, 'application/octet-stream'); + assert.equal(response.data.files['buffer.js'].mimeType, 'application/javascript'); assert.equal(response.data.files['buffer.js'].size, rawData.length); assert.equal(response.data.form.hello, 'hello world๏ผŒ๐Ÿ˜„๐Ÿ˜“'); diff --git a/test/options.followRedirect.test.ts b/test/options.followRedirect.test.ts index 1a399751..26b1c58c 100644 --- a/test/options.followRedirect.test.ts +++ b/test/options.followRedirect.test.ts @@ -3,7 +3,7 @@ import { describe, it, beforeAll, afterAll } from 'vitest'; import urllib from '../src/index.js'; import { startServer } from './fixtures/server.js'; -describe('options.followRedirect.test.js', () => { +describe('options.followRedirect.test.ts', () => { let close: any; let _url: string; beforeAll(async () => { @@ -79,14 +79,14 @@ describe('options.followRedirect.test.js', () => { assert.equal(requestUrls.length, 2); }); - it('should redirect `location: /redirec-full-301-to-url`', async () => { + it('should redirect `location: /redirect-full-301-to-url`', async () => { const requestURL = `${_url}redirect-full-301`; - const { data, res, redirected, url, requestUrls } = await urllib.request(requestURL, { + const { data, res, redirected, url, requestUrls } = await urllib.request(requestURL, { followRedirect: true, }); - // console.log(res.headers); + // console.log(res.headers, res.status); assert.equal(res.statusCode, 200); - assert((data as Buffer).length > 100); + assert(data.length > 100); assert(redirected); assert.equal(url, `${_url}redirect-full-301-to-url`); assert.equal(requestUrls.length, 2); From 1a718481b346ac9372d0b2a09a84faaf565230bf Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Thu, 28 Nov 2024 23:55:03 +0800 Subject: [PATCH 03/10] fix test --- test/diagnostics_channel.test.ts | 4 +++- test/options.files.test.ts | 3 ++- test/options.followRedirect.test.ts | 21 +++++++++++++++++++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/test/diagnostics_channel.test.ts b/test/diagnostics_channel.test.ts index 79583b45..d04ec466 100644 --- a/test/diagnostics_channel.test.ts +++ b/test/diagnostics_channel.test.ts @@ -43,7 +43,9 @@ describe('diagnostics_channel.test.ts', () => { } } } - let opaque = request[kHandler].opts?.opaque ?? request[kHandler].opaque; + const handler = request[kHandler]; + let opaque = handler.opaque || handler.opts?.opaque; + assert(opaque); opaque = opaque[symbols.kRequestOriginalOpaque]; if (opaque && name === 'undici:client:sendHeaders' && socket) { socket[kRequests]++; diff --git a/test/options.files.test.ts b/test/options.files.test.ts index 0119a644..d2132243 100644 --- a/test/options.files.test.ts +++ b/test/options.files.test.ts @@ -215,6 +215,7 @@ describe('options.files.test.ts', () => { const stat = await fs.stat(__filename); const largeFormValue = await fs.readFile(__filename, 'utf-8'); const txtEmoji = path.join(__dirname, 'fixtures', '๐Ÿ˜„foo๐Ÿ˜ญ.txt'); + const txtEmojiStat = await fs.stat(txtEmoji); const txt = path.join(__dirname, 'fixtures', 'foo.txt'); const txtValue = await fs.readFile(txt, 'utf-8'); const response = await urllib.request(`${_url}multipart`, { @@ -237,7 +238,7 @@ describe('options.files.test.ts', () => { assert.equal(response.data.files.file.size, stat.size); assert.equal(response.data.files.file1.filename, '๐Ÿ˜„foo๐Ÿ˜ญ.txt'); assert.equal(response.data.files.file1.mimeType, 'text/plain'); - assert.equal(response.data.files.file1.size, 24); + assert.equal(response.data.files.file1.size, txtEmojiStat.size); assert.equal(response.data.form.hello, 'hello world๏ผŒ๐Ÿ˜„๐Ÿ˜“'); assert.equal( JSON.stringify(decodeURIComponent(response.data.form.txtValue)), diff --git a/test/options.followRedirect.test.ts b/test/options.followRedirect.test.ts index 26b1c58c..48351834 100644 --- a/test/options.followRedirect.test.ts +++ b/test/options.followRedirect.test.ts @@ -1,6 +1,7 @@ import { strict as assert } from 'node:assert'; import { describe, it, beforeAll, afterAll } from 'vitest'; import urllib from '../src/index.js'; +import { HttpClient } from '../src/index.js'; import { startServer } from './fixtures/server.js'; describe('options.followRedirect.test.ts', () => { @@ -79,15 +80,31 @@ describe('options.followRedirect.test.ts', () => { assert.equal(requestUrls.length, 2); }); - it('should redirect `location: /redirect-full-301-to-url`', async () => { + it('should urllib.redirect `location: /redirect-full-301-to-url`', async () => { const requestURL = `${_url}redirect-full-301`; const { data, res, redirected, url, requestUrls } = await urllib.request(requestURL, { followRedirect: true, }); // console.log(res.headers, res.status); assert.equal(res.statusCode, 200); + // console.log(data.toString()); assert(data.length > 100); - assert(redirected); + assert.equal(redirected, true); + assert.equal(url, `${_url}redirect-full-301-to-url`); + assert.equal(requestUrls.length, 2); + }); + + it('should httpClient.redirect `location: /redirect-full-301-to-url`', async () => { + const requestURL = `${_url}redirect-full-301`; + const httpClient = new HttpClient(); + const { data, res, redirected, url, requestUrls } = await httpClient.request(requestURL, { + followRedirect: true, + }); + // console.log(res.headers, res.status); + assert.equal(res.statusCode, 200); + // console.log(data.toString()); + assert(data.length > 100); + assert.equal(redirected, true); assert.equal(url, `${_url}redirect-full-301-to-url`); assert.equal(requestUrls.length, 2); }); From 055b1ec393009ace79413bedf484171cd389b84e Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Fri, 29 Nov 2024 00:02:26 +0800 Subject: [PATCH 04/10] f --- test/index.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/index.test.ts b/test/index.test.ts index 9a403c2f..b0ef86bc 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -148,7 +148,9 @@ describe('index.test.ts', () => { assert.equal(err.res.status, 200); assert.equal(err.name, 'HTTPParserError'); assert.equal(err.message, 'Response does not match the HTTP/1.1 protocol (Invalid character in chunk size)'); - assert.equal(err.code, 'HPE_INVALID_CHUNK_SIZE'); + if (err.code) { + assert.equal(err.code, 'HPE_INVALID_CHUNK_SIZE'); + } assert.equal(err.data, 'labala'); return true; }); From 0eea26acf6d7465918e62533801601fc382ab54a Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Fri, 29 Nov 2024 14:45:04 +0800 Subject: [PATCH 05/10] refactor: auto redirect on the urllib side --- src/HttpClient.ts | 65 +++++++++++++++++++++++++---------------- test/HttpClient.test.ts | 61 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 25 deletions(-) diff --git a/src/HttpClient.ts b/src/HttpClient.ts index 6d7c3fb8..6c6f4339 100644 --- a/src/HttpClient.ts +++ b/src/HttpClient.ts @@ -136,6 +136,8 @@ export type RequestContext = { retries: number; socketErrorRetries: number; requestStartTime?: number; + redirects: number; + history: string[]; }; export const channels = { @@ -170,6 +172,15 @@ export interface PoolStat { size: number; } +// https://developer.mozilla.org/en-US/docs/Web/HTTP/Redirections +const RedirectStatusCodes = [ + 301, // Moved Permanently + 302, // Found + 303, // See Other + 307, // Temporary Redirect + 308, // Permanent Redirect +]; + export class HttpClient extends EventEmitter { #defaultArgs?: RequestOptions; #dispatcher?: Dispatcher; @@ -274,11 +285,14 @@ export class HttpClient extends EventEmitter { requestContext = { retries: 0, socketErrorRetries: 0, + redirects: 0, + history: [], ...requestContext, }; if (!requestContext.requestStartTime) { requestContext.requestStartTime = performance.now(); } + requestContext.history.push(requestUrl.href); const requestStartTime = requestContext.requestStartTime; // https://developer.chrome.com/docs/devtools/network/reference/?utm_source=devtools#timing-explanation @@ -338,7 +352,7 @@ export class HttpClient extends EventEmitter { aborted: false, rt: 0, keepAliveSocket: true, - requestUrls: [], + requestUrls: requestContext.history, timing, socket: socketInfo, retries: requestContext.retries, @@ -399,10 +413,13 @@ export class HttpClient extends EventEmitter { isStreamingRequest = true; } + let maxRedirects = args.maxRedirects ?? 10; + try { const requestOptions: IUndiciRequestOption = { method, - maxRedirections: args.maxRedirects ?? 10, + // disable undici auto redirect handler + maxRedirections: 0, headersTimeout, headers, bodyTimeout, @@ -417,7 +434,7 @@ export class HttpClient extends EventEmitter { requestOptions.reset = args.reset; } if (args.followRedirect === false) { - requestOptions.maxRedirections = 0; + maxRedirects = 0; } const isGETOrHEAD = requestOptions.method === 'GET' || requestOptions.method === 'HEAD'; @@ -545,8 +562,8 @@ export class HttpClient extends EventEmitter { args.socketErrorRetry = 0; } - debug('Request#%d %s %s, headers: %j, headersTimeout: %s, bodyTimeout: %s, isStreamingRequest: %s, maxRedirections: %s', - requestId, requestOptions.method, requestUrl.href, headers, headersTimeout, bodyTimeout, isStreamingRequest, requestOptions.maxRedirections); + debug('Request#%d %s %s, headers: %j, headersTimeout: %s, bodyTimeout: %s, isStreamingRequest: %s, maxRedirections: %s, redirects: %s', + requestId, requestOptions.method, requestUrl.href, headers, headersTimeout, bodyTimeout, isStreamingRequest, maxRedirects, requestContext.redirects); requestOptions.headers = headers; channels.request.publish({ request: reqMeta, @@ -577,18 +594,6 @@ export class HttpClient extends EventEmitter { response = await undiciRequest(requestUrl, requestOptions as UndiciRequestOption); } } - - const context = response.context as { history: URL[] }; - let lastUrl = ''; - if (context?.history) { - for (const urlObject of context?.history) { - res.requestUrls.push(urlObject.href); - lastUrl = urlObject.href; - } - } else { - res.requestUrls.push(requestUrl.href); - lastUrl = requestUrl.href; - } const contentEncoding = response.headers['content-encoding']; const isCompressedContent = contentEncoding === 'gzip' || contentEncoding === 'br'; @@ -599,6 +604,19 @@ export class HttpClient extends EventEmitter { res.size = parseInt(res.headers['content-length']); } + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Redirections + if (RedirectStatusCodes.includes(res.statusCode) && maxRedirects > 0 && requestContext.redirects < maxRedirects && !isStreamingRequest) { + if (res.headers.location) { + requestContext.redirects++; + const nextUrl = new URL(res.headers.location, requestUrl.href); + // Ensure the response is consumed + await response.body.arrayBuffer(); + debug('Request#%d got response, status: %s, headers: %j, timing: %j, redirect to %s', + requestId, res.status, res.headers, res.timing, nextUrl.href); + return await this.#requestInternal(nextUrl.href, options, requestContext); + } + } + let data: any = null; if (args.dataType === 'stream') { // only auto decompress on request args.compressed = true @@ -650,12 +668,15 @@ export class HttpClient extends EventEmitter { statusCode: res.status, statusText: res.statusText, headers: res.headers, - url: lastUrl, - redirected: res.requestUrls.length > 1, + url: requestUrl.href, + redirected: requestContext.history.length > 1, requestUrls: res.requestUrls, res, }; + debug('Request#%d got response, status: %s, headers: %j, timing: %j', + requestId, res.status, res.headers, res.timing); + if (args.retry > 0 && requestContext.retries < args.retry) { const isRetry = args.isRetry ?? defaultIsRetry; if (isRetry(clientResponse)) { @@ -667,8 +688,6 @@ export class HttpClient extends EventEmitter { } } - debug('Request#%d got response, status: %s, headers: %j, timing: %j', - requestId, res.status, res.headers, res.timing); channels.response.publish({ request: reqMeta, response: res, @@ -715,10 +734,6 @@ export class HttpClient extends EventEmitter { err._rawSocket = err.socket; } err.socket = socketInfo; - // make sure requestUrls not empty - if (res.requestUrls.length === 0) { - res.requestUrls.push(requestUrl.href); - } res.rt = performanceTime(requestStartTime); updateSocketInfo(socketInfo, internalOpaque, rawError); diff --git a/test/HttpClient.test.ts b/test/HttpClient.test.ts index b12347ac..6ae33e80 100644 --- a/test/HttpClient.test.ts +++ b/test/HttpClient.test.ts @@ -170,6 +170,67 @@ describe('HttpClient.test.ts', () => { // console.log(response.res.socket, response.res.timing); assert.equal(response.data, 'hello h2!'); }); + + it('should auto redirect work', async () => { + const server = createSecureServer(pem); + + let count = 0; + server.on('stream', (stream, headers) => { + count++; + // console.log(count, headers); + if (count === 2) { + stream.respond({ + 'content-type': 'text/plain; charset=utf-8', + 'x-custom-h2': 'hello', + location: '/see-other', + ':status': 302, + }); + stream.end(); + return; + } + assert.equal(headers[':method'], 'GET'); + stream.respond({ + 'content-type': 'text/plain; charset=utf-8', + 'x-custom-h2': 'hello', + ':status': 200, + }); + stream.end('hello h2!'); + }); + + server.listen(0); + await once(server, 'listening'); + + const httpClient = new HttpClient({ + allowH2: true, + connect: { + rejectUnauthorized: false, + }, + }); + + const url = `https://localhost:${server.address()!.port}`; + let response = await httpClient.request(url, { + dataType: 'text', + headers: { + 'x-my-header': 'foo', + }, + }); + assert.equal(response.status, 200); + assert.equal(response.headers['x-custom-h2'], 'hello'); + // console.log(response.res.socket, response.res.timing); + assert.equal(response.data, 'hello h2!'); + await sleep(200); + response = await httpClient.request(url, { + dataType: 'text', + headers: { + 'x-my-header': 'foo2', + }, + followRedirect: true, + }); + assert.equal(response.status, 200); + assert.equal(response.headers['x-custom-h2'], 'hello'); + // console.log(response.res.socket, response.res.timing); + assert.equal(response.data, 'hello h2!'); + }); }); describe('clientOptions.defaultArgs', () => { From 12c7b229247be149322d03367396cdefe1f8923d Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Fri, 29 Nov 2024 14:59:27 +0800 Subject: [PATCH 06/10] f --- test/options.files.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/options.files.test.ts b/test/options.files.test.ts index d2132243..2cc88642 100644 --- a/test/options.files.test.ts +++ b/test/options.files.test.ts @@ -36,6 +36,21 @@ describe('options.files.test.ts', () => { assert.equal(response.data.files.file.size, stat.size); }); + it('should upload not exists file throw error', async () => { + const file = path.join(__dirname, 'cjs', 'index.js-not-exists'); + await assert.rejects(async () => { + await urllib.request(`${_url}multipart`, { + files: [ file ], + dataType: 'json', + }); + }, (err: any) => { + assert.equal(err.code, 'ENOENT'); + assert.equal(err.res.status, -1); + assert.equal(err.status, -1); + return true; + }); + }); + it('should upload files = [filepath] success with default POST method', async () => { const file = path.join(__dirname, 'cjs', 'index.js'); const stat = await fs.stat(file); From 0f19706d7cd0a3cd0bd4e1dbb76eefa9bd969e21 Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Fri, 29 Nov 2024 16:38:10 +0800 Subject: [PATCH 07/10] fix redirect on writeStream --- package.json | 4 ++-- scripts/replace_urllib_version.js | 5 +++-- src/HttpClient.ts | 14 ++++++++++---- test/options.followRedirect.test.ts | 25 +++++++++++++++++++++++++ 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index f04963d2..5fb8ee43 100644 --- a/package.json +++ b/package.json @@ -1,8 +1,8 @@ { "name": "urllib", - "version": "4.4.0", + "version": "4.5.0-beta.3", "publishConfig": { - "tag": "latest" + "tag": "beta" }, "description": "Help in opening URLs (mostly HTTP) in a complex world โ€” basic and digest authentication, redirections, timeout and more. Base undici API.", "keywords": [ diff --git a/scripts/replace_urllib_version.js b/scripts/replace_urllib_version.js index ee06f779..eac94165 100644 --- a/scripts/replace_urllib_version.js +++ b/scripts/replace_urllib_version.js @@ -13,8 +13,9 @@ async function main() { for (const file of files) { const content = await fs.readFile(file, 'utf-8'); // replace "const VERSION = 'VERSION';" to "const VERSION = '4.0.0';" - const newContent = content.replace(/const VERSION = 'VERSION';/, match => { - const after = `const VERSION = '${pkg.version}';`; + // "exports.VERSION = 'VERSION';" => "exports.VERSION = '4.0.0';" + const newContent = content.replace(/ = 'VERSION';/, match => { + const after = ` = '${pkg.version}';`; console.log('[%s] %s => %s', file, match, after); return after; }); diff --git a/src/HttpClient.ts b/src/HttpClient.ts index 6c6f4339..95e1c4ad 100644 --- a/src/HttpClient.ts +++ b/src/HttpClient.ts @@ -409,8 +409,9 @@ export class HttpClient extends EventEmitter { // streaming request should disable socketErrorRetry and retry let isStreamingRequest = false; + let isStreamingResponse = false; if (args.dataType === 'stream' || args.writeStream) { - isStreamingRequest = true; + isStreamingResponse = true; } let maxRedirects = args.maxRedirects ?? 10; @@ -560,10 +561,15 @@ export class HttpClient extends EventEmitter { if (isStreamingRequest) { args.retry = 0; args.socketErrorRetry = 0; + maxRedirects = 0; + } + if (isStreamingResponse) { + args.retry = 0; + args.socketErrorRetry = 0; } - debug('Request#%d %s %s, headers: %j, headersTimeout: %s, bodyTimeout: %s, isStreamingRequest: %s, maxRedirections: %s, redirects: %s', - requestId, requestOptions.method, requestUrl.href, headers, headersTimeout, bodyTimeout, isStreamingRequest, maxRedirects, requestContext.redirects); + debug('Request#%d %s %s, headers: %j, headersTimeout: %s, bodyTimeout: %s, isStreamingRequest: %s, isStreamingResponse: %s, maxRedirections: %s, redirects: %s', + requestId, requestOptions.method, requestUrl.href, headers, headersTimeout, bodyTimeout, isStreamingRequest, isStreamingResponse, maxRedirects, requestContext.redirects); requestOptions.headers = headers; channels.request.publish({ request: reqMeta, @@ -605,7 +611,7 @@ export class HttpClient extends EventEmitter { } // https://developer.mozilla.org/en-US/docs/Web/HTTP/Redirections - if (RedirectStatusCodes.includes(res.statusCode) && maxRedirects > 0 && requestContext.redirects < maxRedirects && !isStreamingRequest) { + if (RedirectStatusCodes.includes(res.statusCode) && maxRedirects > 0 && requestContext.redirects < maxRedirects) { if (res.headers.location) { requestContext.redirects++; const nextUrl = new URL(res.headers.location, requestUrl.href); diff --git a/test/options.followRedirect.test.ts b/test/options.followRedirect.test.ts index 48351834..2dd6c172 100644 --- a/test/options.followRedirect.test.ts +++ b/test/options.followRedirect.test.ts @@ -1,20 +1,29 @@ import { strict as assert } from 'node:assert'; +import { createWriteStream } from 'node:fs'; import { describe, it, beforeAll, afterAll } from 'vitest'; import urllib from '../src/index.js'; import { HttpClient } from '../src/index.js'; import { startServer } from './fixtures/server.js'; +import { createTempfile } from './utils.js'; describe('options.followRedirect.test.ts', () => { let close: any; let _url: string; + let tmpfile: string; + let cleanup: any; + beforeAll(async () => { const { closeServer, url } = await startServer(); close = closeServer; _url = url; + const item = await createTempfile(); + tmpfile = item.tmpfile; + cleanup = item.cleanup; }); afterAll(async () => { await close(); + await cleanup(); }); it('should redirect `location: /redirect-to-url`', async () => { @@ -30,6 +39,22 @@ describe('options.followRedirect.test.ts', () => { assert.equal(response.requestUrls.length, 2); }); + it('should follow redirect on writeStream ', async () => { + const requestURL = `${_url}redirect`; + const httpClient = new HttpClient({ + allowH2: true, + }); + const response = await httpClient.request(requestURL, { + followRedirect: true, + writeStream: createWriteStream(tmpfile), + }); + assert.equal(response.res.statusCode, 200); + assert.equal(response.statusCode, response.res.statusCode); + assert.equal(response.statusText, 'OK'); + assert.equal(response.data, null); + assert.equal(response.requestUrls.length, 2); + }); + it('should followRedirect is true and maxRedirects default up to 10', async () => { const requestURL = `${_url}redirect-deadlock`; const response = await urllib.request(requestURL, { From ee3bef072d21db4faad751171deb0f801a3032c0 Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Fri, 29 Nov 2024 16:39:30 +0800 Subject: [PATCH 08/10] f --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5fb8ee43..545c05c8 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "urllib", "version": "4.5.0-beta.3", "publishConfig": { - "tag": "beta" + "tag": "latest" }, "description": "Help in opening URLs (mostly HTTP) in a complex world โ€” basic and digest authentication, redirections, timeout and more. Base undici API.", "keywords": [ From 7ae86d3ef57ef3bd9ec0169bbdafda35b33693a7 Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Fri, 29 Nov 2024 22:26:42 +0800 Subject: [PATCH 09/10] support timeout on h2 --- src/HttpClient.ts | 2 ++ test/options.timeout.test.ts | 41 +++++++++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/HttpClient.ts b/src/HttpClient.ts index 95e1c4ad..d084b4d8 100644 --- a/src/HttpClient.ts +++ b/src/HttpClient.ts @@ -720,6 +720,8 @@ export class HttpClient extends EventEmitter { err = new HttpClientRequestTimeoutError(headersTimeout, { cause: err }); } else if (err.name === 'BodyTimeoutError') { err = new HttpClientRequestTimeoutError(bodyTimeout, { cause: err }); + } else if (err.name === 'InformationalError' && err.message.includes('stream timeout')) { + err = new HttpClientRequestTimeoutError(bodyTimeout, { cause: err }); } else if (err.code === 'UND_ERR_CONNECT_TIMEOUT') { err = new HttpClientConnectTimeoutError(err.message, err.code, { cause: err }); } else if (err.code === 'UND_ERR_SOCKET' || err.code === 'ECONNRESET') { diff --git a/test/options.timeout.test.ts b/test/options.timeout.test.ts index b1e78356..04bd4d8f 100644 --- a/test/options.timeout.test.ts +++ b/test/options.timeout.test.ts @@ -1,6 +1,9 @@ import { strict as assert } from 'node:assert'; +import { createSecureServer } from 'node:http2'; +import { once } from 'node:events'; +import pem from 'https-pem'; import { describe, it, beforeAll, afterAll } from 'vitest'; -import urllib, { HttpClientRequestTimeoutError } from '../src/index.js'; +import urllib, { HttpClientRequestTimeoutError, HttpClient } from '../src/index.js'; import { startServer } from './fixtures/server.js'; describe('options.timeout.test.ts', () => { @@ -36,6 +39,42 @@ describe('options.timeout.test.ts', () => { }); }); + it('should timeout on h2', async () => { + const httpClient = new HttpClient({ + allowH2: true, + connect: { + rejectUnauthorized: false, + }, + }); + const server = createSecureServer(pem); + + server.on('stream', () => { + // wait for timeout + }); + + server.listen(0); + await once(server, 'listening'); + + const url = `https://localhost:${server.address()!.port}`; + await assert.rejects(async () => { + await httpClient.request(url, { + timeout: 10, + }); + }, (err: any) => { + // console.error(err); + assert.equal(err.name, 'HttpClientRequestTimeoutError'); + assert.equal(err.message, 'Request timeout for 10 ms'); + assert.equal(err.cause.name, 'InformationalError'); + assert.equal(err.cause.message, 'HTTP/2: "stream timeout after 10"'); + assert.equal(err.cause.code, 'UND_ERR_INFO'); + + assert.equal(err.res.status, -1); + assert(err.res.rt > 10, `actual ${err.res.rt}`); + assert.equal(typeof err.res.rt, 'number'); + return true; + }); + }); + it('should BodyTimeout throw error', async () => { await assert.rejects(async () => { await urllib.request(`${_url}mock-bytes?timeout=2000`, { From ce4af6b629e9581b6fd2b299b65ff8eccff0d61d Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Sat, 30 Nov 2024 10:55:51 +0800 Subject: [PATCH 10/10] f --- src/HttpClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HttpClient.ts b/src/HttpClient.ts index d084b4d8..1d0f0b1e 100644 --- a/src/HttpClient.ts +++ b/src/HttpClient.ts @@ -24,7 +24,6 @@ import { getGlobalDispatcher, Pool, } from 'undici'; -import { FormData } from './FormData.js'; // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore import undiciSymbols from 'undici/lib/core/symbols.js'; @@ -32,6 +31,7 @@ import mime from 'mime-types'; import qs from 'qs'; // Compatible with old style formstream import FormStream from 'formstream'; +import { FormData } from './FormData.js'; import { HttpAgent, CheckAddressFunction } from './HttpAgent.js'; import type { IncomingHttpHeaders } from './IncomingHttpHeaders.js'; import { RequestURL, RequestOptions, HttpMethod, RequestMeta } from './Request.js';