Skip to content

Commit

Permalink
fix: adpater http/2 agent on diagnosticsChannel (#511)
Browse files Browse the repository at this point in the history
  • Loading branch information
fengmk2 authored Jun 1, 2024
1 parent 00e196a commit d565da2
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 17 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ name: CI
on:
push:
branches: [ master ]

pull_request:
branches: [ master ]

Expand All @@ -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 }}
2 changes: 0 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,3 @@ jobs:
secrets:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
GIT_TOKEN: ${{ secrets.GIT_TOKEN }}
with:
install: 'npx npminstall'
17 changes: 12 additions & 5 deletions src/diagnosticsChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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]);

Expand Down
4 changes: 2 additions & 2 deletions test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
});
});

Expand Down Expand Up @@ -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 () => {
Expand Down
8 changes: 4 additions & 4 deletions test/keep-alive-header.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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 }
Expand Down
17 changes: 16 additions & 1 deletion test/options.dispatcher.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -62,4 +62,19 @@ describe('options.dispatcher.test.ts', () => {
assert.equal(response.data, '<h1>hello</h1>');
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');
});
});

0 comments on commit d565da2

Please sign in to comment.