Skip to content

Commit b7ea39d

Browse files
RaisinTentargos
authored andcommitted
http2: report sent headers object in client stream dcs
This change improves diagnosis by reporting the headers object that is actually sent rather than the original input headers in the following diagnostics channels: - 'http2.client.stream.created' - 'http2.client.stream.start' Signed-off-by: Darshan Sen <[email protected]> PR-URL: #59419 Reviewed-By: Luigi Pinca <[email protected]>
1 parent 7f457f8 commit b7ea39d

File tree

3 files changed

+63
-12
lines changed

3 files changed

+63
-12
lines changed

lib/internal/http2/core.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ const deprecateWeight = deprecateProperty('weight',
760760
// When a ClientHttp2Session is first created, the socket may not yet be
761761
// connected. If request() is called during this time, the actual request
762762
// will be deferred until the socket is ready to go.
763-
function requestOnConnect(headersList, headersParam, options) {
763+
function requestOnConnect(headersList, options) {
764764
const session = this[kSession];
765765

766766
// At this point, the stream should have already been destroyed during
@@ -824,7 +824,7 @@ function requestOnConnect(headersList, headersParam, options) {
824824
if (onClientStreamStartChannel.hasSubscribers) {
825825
onClientStreamStartChannel.publish({
826826
stream: this,
827-
headers: headersParam,
827+
headers: this.sentHeaders,
828828
});
829829
}
830830
}
@@ -1888,7 +1888,7 @@ class ClientHttp2Session extends Http2Session {
18881888
}
18891889
}
18901890

1891-
const onConnect = reqAsync.bind(requestOnConnect.bind(stream, headersList, headersParam, options));
1891+
const onConnect = reqAsync.bind(requestOnConnect.bind(stream, headersList, options));
18921892
if (this.connecting) {
18931893
if (this[kPendingRequestCalls] !== null) {
18941894
this[kPendingRequestCalls].push(onConnect);
@@ -1906,7 +1906,7 @@ class ClientHttp2Session extends Http2Session {
19061906
if (onClientStreamCreatedChannel.hasSubscribers) {
19071907
onClientStreamCreatedChannel.publish({
19081908
stream,
1909-
headers: headersParam,
1909+
headers: stream.sentHeaders,
19101910
});
19111911
}
19121912

test/parallel/test-diagnostics-channel-http2-client-stream-created.js

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,36 +18,61 @@ const { Duplex } = require('stream');
1818

1919
const clientHttp2StreamCreationCount = 2;
2020

21+
let countdown;
22+
let port;
23+
2124
dc.subscribe('http2.client.stream.created', common.mustCall(({ stream, headers }) => {
2225
// Since ClientHttp2Stream is not exported from any module, this just checks
2326
// if the stream is an instance of Duplex and the constructor name is
2427
// 'ClientHttp2Stream'.
2528
assert.ok(stream instanceof Duplex);
2629
assert.strictEqual(stream.constructor.name, 'ClientHttp2Stream');
2730
assert.ok(headers && !Array.isArray(headers) && typeof headers === 'object');
31+
if (countdown.remaining === clientHttp2StreamCreationCount) {
32+
// The request stream headers.
33+
assert.deepStrictEqual(headers, {
34+
'__proto__': null,
35+
':method': 'GET',
36+
':authority': `localhost:${port}`,
37+
':scheme': 'http',
38+
':path': '/',
39+
'requestHeader': 'requestValue',
40+
});
41+
} else {
42+
// The push stream headers.
43+
assert.deepStrictEqual(headers, {
44+
'__proto__': null,
45+
':method': 'GET',
46+
':authority': `localhost:${port}`,
47+
':scheme': 'http',
48+
':path': '/',
49+
[http2.sensitiveHeaders]: [],
50+
'pushheader': 'pushValue',
51+
});
52+
}
2853
}, clientHttp2StreamCreationCount));
2954

3055
const server = http2.createServer();
3156
server.on('stream', common.mustCall((stream) => {
3257
stream.respond();
3358
stream.end();
3459

35-
stream.pushStream({}, common.mustSucceed((pushStream) => {
60+
stream.pushStream({ 'pushHeader': 'pushValue' }, common.mustSucceed((pushStream) => {
3661
pushStream.respond();
3762
pushStream.end();
3863
}, 1));
3964
}, 1));
4065

4166
server.listen(0, common.mustCall(() => {
42-
const port = server.address().port;
67+
port = server.address().port;
4368
const client = http2.connect(`http://localhost:${port}`);
4469

45-
const countdown = new Countdown(clientHttp2StreamCreationCount, () => {
70+
countdown = new Countdown(clientHttp2StreamCreationCount, () => {
4671
client.close();
4772
server.close();
4873
});
4974

50-
const stream = client.request({});
75+
const stream = client.request(['requestHeader', 'requestValue']);
5176
stream.on('response', common.mustCall(() => {
5277
countdown.dec();
5378
}));

test/parallel/test-diagnostics-channel-http2-client-stream-start.js

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,35 +18,61 @@ const { Duplex } = require('stream');
1818

1919
const clientHttp2StreamStartCount = 2;
2020

21+
let countdown;
22+
let port;
23+
2124
dc.subscribe('http2.client.stream.start', common.mustCall(({ stream, headers }) => {
2225
// Since ClientHttp2Stream is not exported from any module, this just checks
2326
// if the stream is an instance of Duplex.
2427
assert.ok(stream instanceof Duplex);
2528
assert.strictEqual(stream.constructor.name, 'ClientHttp2Stream');
2629
assert.ok(headers && !Array.isArray(headers) && typeof headers === 'object');
30+
31+
if (countdown.remaining === clientHttp2StreamStartCount) {
32+
// The request stream headers.
33+
assert.deepStrictEqual(headers, {
34+
'__proto__': null,
35+
':method': 'GET',
36+
':authority': `localhost:${port}`,
37+
':scheme': 'http',
38+
':path': '/',
39+
'requestHeader': 'requestValue',
40+
});
41+
} else {
42+
// The push stream headers.
43+
assert.deepStrictEqual(headers, {
44+
'__proto__': null,
45+
':method': 'GET',
46+
':authority': `localhost:${port}`,
47+
':scheme': 'http',
48+
':path': '/',
49+
[http2.sensitiveHeaders]: [],
50+
'pushheader': 'pushValue',
51+
});
52+
}
2753
}, clientHttp2StreamStartCount));
2854

2955
const server = http2.createServer();
3056
server.on('stream', common.mustCall((stream) => {
3157
stream.respond();
3258
stream.end();
3359

34-
stream.pushStream({}, common.mustSucceed((pushStream) => {
60+
stream.pushStream({ 'pushHeader': 'pushValue' }, common.mustSucceed((pushStream) => {
3561
pushStream.respond();
3662
pushStream.end();
3763
}, 1));
3864
}, 1));
3965

4066
server.listen(0, common.mustCall(() => {
41-
const port = server.address().port;
67+
port = server.address().port;
4268
const client = http2.connect(`http://localhost:${port}`);
4369

44-
const countdown = new Countdown(clientHttp2StreamStartCount, () => {
70+
countdown = new Countdown(clientHttp2StreamStartCount, () => {
4571
client.close();
4672
server.close();
4773
});
4874

49-
const stream = client.request({});
75+
const stream = client.request(['requestHeader', 'requestValue']);
5076
stream.on('response', common.mustCall(() => {
5177
countdown.dec();
5278
}));

0 commit comments

Comments
 (0)