From d565da230bb5be0714e1fcaf36db2f30844c29f2 Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Sat, 1 Jun 2024 11:07:08 +0800 Subject: [PATCH] fix: adpater http/2 agent on diagnosticsChannel (#511) https://github.com/node-modules/urllib/issues/510 --- .github/workflows/nodejs.yml | 6 +++--- .github/workflows/release.yml | 2 -- src/diagnosticsChannel.ts | 17 ++++++++++++----- test/index.test.ts | 4 ++-- test/keep-alive-header.test.ts | 8 ++++---- test/options.dispatcher.test.ts | 17 ++++++++++++++++- 6 files changed, 37 insertions(+), 17 deletions(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index e9b172c9..9e51f0d3 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -3,7 +3,6 @@ name: CI on: push: branches: [ master ] - pull_request: branches: [ master ] @@ -13,5 +12,6 @@ jobs: uses: node-modules/github-actions/.github/workflows/node-test.yml@master with: os: 'ubuntu-latest, macos-latest, windows-latest' - version: '14.19.3, 14, 16, 18, 20, 21' - install: 'npx npminstall' + version: '14.19.3, 14, 16, 18, 20, 22' + secrets: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e83042eb..1c6cbb18 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -11,5 +11,3 @@ jobs: secrets: NPM_TOKEN: ${{ secrets.NPM_TOKEN }} GIT_TOKEN: ${{ secrets.GIT_TOKEN }} - with: - install: 'npx npminstall' diff --git a/src/diagnosticsChannel.ts b/src/diagnosticsChannel.ts index 51fe618f..97780e00 100644 --- a/src/diagnosticsChannel.ts +++ b/src/diagnosticsChannel.ts @@ -184,10 +184,15 @@ export function initDiagnosticsChannel() { // get socket from opaque const socket = opaque[symbols.kRequestSocket]; - socket[symbols.kHandledResponses]++; - debug('[%s] Request#%d get %s response headers on Socket#%d (handled %d responses, sock: %o)', - name, opaque[symbols.kRequestId], response.statusCode, socket[symbols.kSocketId], socket[symbols.kHandledResponses], - formatSocket(socket)); + if (socket) { + socket[symbols.kHandledResponses]++; + debug('[%s] Request#%d get %s response headers on Socket#%d (handled %d responses, sock: %o)', + name, opaque[symbols.kRequestId], response.statusCode, socket[symbols.kSocketId], socket[symbols.kHandledResponses], + formatSocket(socket)); + } else { + debug('[%s] Request#%d get %s response headers on Unknown Socket', + name, opaque[symbols.kRequestId], response.statusCode); + } if (!opaque[symbols.kEnableRequestTiming]) return; opaque[symbols.kRequestTiming].waiting = performanceTime(opaque[symbols.kRequestStartTime]); @@ -197,7 +202,9 @@ export function initDiagnosticsChannel() { subscribe('undici:request:trailers', (message, name) => { const { request } = message as DiagnosticsChannel.RequestTrailersMessage; const opaque = getRequestOpaque(request, kHandler); - if (!opaque || !opaque[symbols.kRequestId]) return; + if (!opaque || !opaque[symbols.kRequestId]) { + return; + } debug('[%s] Request#%d get response body and trailers', name, opaque[symbols.kRequestId]); diff --git a/test/index.test.ts b/test/index.test.ts index 4a128179..90a677c0 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -32,7 +32,7 @@ describe('index.test.ts', () => { assert.equal(response.url, `${_url}html`); assert(!response.redirected); assert.equal(getDefaultHttpClient(), getDefaultHttpClient()); - console.log('stats %o', getDefaultHttpClient().getDispatcherPoolStats()); + // console.log('stats %o', getDefaultHttpClient().getDispatcherPoolStats()); }); }); @@ -76,7 +76,7 @@ describe('index.test.ts', () => { dataType: 'json', }); assert.equal(response.status, 200); - console.log(response.data.hello); + // console.log(response.data.hello); }); it('should curl alias to request', async () => { diff --git a/test/keep-alive-header.test.ts b/test/keep-alive-header.test.ts index 61e2f8c9..3d797062 100644 --- a/test/keep-alive-header.test.ts +++ b/test/keep-alive-header.test.ts @@ -31,7 +31,7 @@ describe('keep-alive-header.test.ts', () => { count++; try { const task = httpClient.request(_url); - console.log('after request stats: %o', httpClient.getDispatcherPoolStats()); + // console.log('after request stats: %o', httpClient.getDispatcherPoolStats()); if (httpClient.getDispatcherPoolStats()[origin]) { if (!(nodeMajorVersion() === 14 || isWindows())) { // ignore node = 14 & windows @@ -40,7 +40,7 @@ describe('keep-alive-header.test.ts', () => { } } let response = await task; - console.log('after response stats: %o', httpClient.getDispatcherPoolStats()); + // console.log('after response stats: %o', httpClient.getDispatcherPoolStats()); assert.equal(httpClient.getDispatcherPoolStats()[origin].pending, 0); assert.equal(httpClient.getDispatcherPoolStats()[origin].connected, 1); // console.log(response.res.socket); @@ -129,12 +129,12 @@ describe('keep-alive-header.test.ts', () => { assert.equal(response.headers.connection, 'keep-alive'); assert.equal(response.headers['keep-alive'], 'timeout=2'); assert(parseInt(response.headers['x-requests-persocket'] as string) > 1); - console.log('before sleep stats: %o', httpClient.getDispatcherPoolStats()); + // console.log('before sleep stats: %o', httpClient.getDispatcherPoolStats()); // { connected: 2, free: 1, pending: 0, queued: 0, running: 0, size: 0 } assert.equal(httpClient.getDispatcherPoolStats()[origin].connected, 2); assert.equal(httpClient.getDispatcherPoolStats()[origin].free, 1); await sleep(keepAliveTimeout); - console.log('after sleep stats: %o', httpClient.getDispatcherPoolStats()); + // console.log('after sleep stats: %o', httpClient.getDispatcherPoolStats()); // clients maybe all gone => after sleep stats: {} // { connected: 0, free: 0, pending: 0, queued: 0, running: 0, size: 0 } // { connected: 1, free: 1, pending: 0, queued: 0, running: 0, size: 0 } diff --git a/test/options.dispatcher.test.ts b/test/options.dispatcher.test.ts index 0571f870..bde1e00e 100644 --- a/test/options.dispatcher.test.ts +++ b/test/options.dispatcher.test.ts @@ -1,7 +1,7 @@ import { strict as assert } from 'node:assert'; import { describe, it, beforeAll, afterAll } from 'vitest'; import setup from 'proxy'; -import { request, ProxyAgent, getGlobalDispatcher, setGlobalDispatcher } from '../src'; +import { request, ProxyAgent, getGlobalDispatcher, setGlobalDispatcher, Agent } from '../src'; import { startServer } from './fixtures/server'; describe('options.dispatcher.test.ts', () => { @@ -62,4 +62,19 @@ describe('options.dispatcher.test.ts', () => { assert.equal(response.data, '

hello

'); setGlobalDispatcher(agent); }); + + it('should work with http/2 dispatcher', async () => { + // https://github.com/nodejs/undici/issues/2750#issuecomment-1941009554 + const agent = new Agent({ + allowH2: true, + }); + assert(agent); + const response = await request('https://registry.npmmirror.com', { + dataType: 'json', + timing: true, + dispatcher: agent, + }); + assert.equal(response.status, 200); + assert.equal(response.headers['content-type'], 'application/json; charset=utf-8'); + }); });