Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 4d4fb51

Browse filesBrowse files
RaisinTenrichardlau
RaisinTen
authored and
richardlau
committed
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 <raisinten@gmail.com> PR-URL: #59419 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent 162ac93 commit 4d4fb51
Copy full SHA for 4d4fb51

File tree

Expand file treeCollapse file tree

3 files changed

+63
-12
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+63
-12
lines changed
Open diff view settings
Collapse file

‎lib/internal/http2/core.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/core.js
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,7 @@ function onGoawayData(code, lastStreamID, buf) {
752752
// When a ClientHttp2Session is first created, the socket may not yet be
753753
// connected. If request() is called during this time, the actual request
754754
// will be deferred until the socket is ready to go.
755-
function requestOnConnect(headersList, headersParam, options) {
755+
function requestOnConnect(headersList, options) {
756756
const session = this[kSession];
757757

758758
// At this point, the stream should have already been destroyed during
@@ -814,7 +814,7 @@ function requestOnConnect(headersList, headersParam, options) {
814814
if (onClientStreamStartChannel.hasSubscribers) {
815815
onClientStreamStartChannel.publish({
816816
stream: this,
817-
headers: headersParam,
817+
headers: this.sentHeaders,
818818
});
819819
}
820820
}
@@ -1894,7 +1894,7 @@ class ClientHttp2Session extends Http2Session {
18941894
}
18951895
}
18961896

1897-
const onConnect = reqAsync.bind(requestOnConnect.bind(stream, headersList, headersParam, options));
1897+
const onConnect = reqAsync.bind(requestOnConnect.bind(stream, headersList, options));
18981898
if (this.connecting) {
18991899
if (this[kPendingRequestCalls] !== null) {
19001900
this[kPendingRequestCalls].push(onConnect);
@@ -1912,7 +1912,7 @@ class ClientHttp2Session extends Http2Session {
19121912
if (onClientStreamCreatedChannel.hasSubscribers) {
19131913
onClientStreamCreatedChannel.publish({
19141914
stream,
1915-
headers: headersParam,
1915+
headers: stream.sentHeaders,
19161916
});
19171917
}
19181918

Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-diagnostics-channel-http2-client-stream-created.js
+29-4Lines 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
}));
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-diagnostics-channel-http2-client-stream-start.js
+30-4Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.