Skip to content

Commit 58d8c9e

Browse files
addaleaxtargos
authored andcommitted
http2: improve JS-side debug logging
DRY up the `debug()` calls, and in particular, avoid building template strings before we know whether we need to. PR-URL: #29122 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent bfeb5fc commit 58d8c9e

File tree

1 file changed

+58
-54
lines changed

1 file changed

+58
-54
lines changed

lib/internal/http2/core.js

Lines changed: 58 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,26 @@ const { _connectionListener: httpConnectionListener } = http;
139139
const debug = require('internal/util/debuglog').debuglog('http2');
140140
const { getOptionValue } = require('internal/options');
141141

142+
// TODO(addaleax): See if this can be made more efficient by figuring out
143+
// whether debugging is enabled before we perform any further steps. Currently,
144+
// this seems pretty fast, though.
145+
function debugStream(id, sessionType, message, ...args) {
146+
debug('Http2Stream %s [Http2Session %s]: ' + message,
147+
id, sessionName(sessionType), ...args);
148+
}
149+
150+
function debugStreamObj(stream, message, ...args) {
151+
debugStream(stream[kID], stream[kSession][kType], ...args);
152+
}
153+
154+
function debugSession(sessionType, message, ...args) {
155+
debug('Http2Session %s: ' + message, sessionName(sessionType), ...args);
156+
}
157+
158+
function debugSessionObj(session, message, ...args) {
159+
debugSession(session[kType], message, ...args);
160+
}
161+
142162
const kMaxFrameSize = (2 ** 24) - 1;
143163
const kMaxInt = (2 ** 32) - 1;
144164
const kMaxStreams = (2 ** 31) - 1;
@@ -260,8 +280,7 @@ function onSessionHeaders(handle, id, cat, flags, headers) {
260280

261281
const type = session[kType];
262282
session[kUpdateTimer]();
263-
debug(`Http2Stream ${id} [Http2Session ` +
264-
`${sessionName(type)}]: headers received`);
283+
debugStream(id, type, 'headers received');
265284
const streams = session[kState].streams;
266285

267286
const endOfStream = !!(flags & NGHTTP2_FLAG_END_STREAM);
@@ -321,8 +340,7 @@ function onSessionHeaders(handle, id, cat, flags, headers) {
321340
const originSet = session[kState].originSet = initOriginSet(session);
322341
originSet.delete(stream[kOrigin]);
323342
}
324-
debug(`Http2Stream ${id} [Http2Session ` +
325-
`${sessionName(type)}]: emitting stream '${event}' event`);
343+
debugStream(id, type, "emitting stream '%s' event", event);
326344
process.nextTick(emit, stream, event, obj, flags, headers);
327345
}
328346
if (endOfStream) {
@@ -363,7 +381,7 @@ function onPing(payload) {
363381
if (session.destroyed)
364382
return;
365383
session[kUpdateTimer]();
366-
debug(`Http2Session ${sessionName(session[kType])}: new ping received`);
384+
debugSessionObj(session, 'new ping received');
367385
session.emit('ping', payload);
368386
}
369387

@@ -378,8 +396,7 @@ function onStreamClose(code) {
378396
if (!stream || stream.destroyed)
379397
return false;
380398

381-
debug(`Http2Stream ${stream[kID]} [Http2Session ` +
382-
`${sessionName(stream[kSession][kType])}]: closed with code ${code}`);
399+
debugStreamObj(stream, 'closed with code %d', code);
383400

384401
if (!stream.closed)
385402
closeStream(stream, code, kNoRstStream);
@@ -416,7 +433,7 @@ function onSettings() {
416433
if (session.destroyed)
417434
return;
418435
session[kUpdateTimer]();
419-
debug(`Http2Session ${sessionName(session[kType])}: new settings received`);
436+
debugSessionObj(session, 'new settings received');
420437
session[kRemoteSettings] = undefined;
421438
session.emit('remoteSettings', session.remoteSettings);
422439
}
@@ -428,9 +445,9 @@ function onPriority(id, parent, weight, exclusive) {
428445
const session = this[kOwner];
429446
if (session.destroyed)
430447
return;
431-
debug(`Http2Stream ${id} [Http2Session ` +
432-
`${sessionName(session[kType])}]: priority [parent: ${parent}, ` +
433-
`weight: ${weight}, exclusive: ${exclusive}]`);
448+
debugStream(id, session[kType],
449+
'priority [parent: %d, weight: %d, exclusive: %s]',
450+
parent, weight, exclusive);
434451
const emitter = session[kState].streams.get(id) || session;
435452
if (!emitter.destroyed) {
436453
emitter[kUpdateTimer]();
@@ -444,8 +461,8 @@ function onFrameError(id, type, code) {
444461
const session = this[kOwner];
445462
if (session.destroyed)
446463
return;
447-
debug(`Http2Session ${sessionName(session[kType])}: error sending frame ` +
448-
`type ${type} on stream ${id}, code: ${code}`);
464+
debugSessionObj(session, 'error sending frame type %d on stream %d, code: %d',
465+
type, id, code);
449466
const emitter = session[kState].streams.get(id) || session;
450467
emitter[kUpdateTimer]();
451468
emitter.emit('frameError', type, code, id);
@@ -455,8 +472,8 @@ function onAltSvc(stream, origin, alt) {
455472
const session = this[kOwner];
456473
if (session.destroyed)
457474
return;
458-
debug(`Http2Session ${sessionName(session[kType])}: altsvc received: ` +
459-
`stream: ${stream}, origin: ${origin}, alt: ${alt}`);
475+
debugSessionObj(session, 'altsvc received: stream: %d, origin: %s, alt: %s',
476+
stream, origin, alt);
460477
session[kUpdateTimer]();
461478
session.emit('altsvc', alt, origin, stream);
462479
}
@@ -483,8 +500,7 @@ function onOrigin(origins) {
483500
const session = this[kOwner];
484501
if (session.destroyed)
485502
return;
486-
debug('Http2Session %s: origin received: %j',
487-
sessionName(session[kType]), origins);
503+
debugSessionObj(session, 'origin received: %j', origins);
488504
session[kUpdateTimer]();
489505
if (!session.encrypted || session.destroyed)
490506
return undefined;
@@ -504,8 +520,8 @@ function onGoawayData(code, lastStreamID, buf) {
504520
const session = this[kOwner];
505521
if (session.destroyed)
506522
return;
507-
debug(`Http2Session ${sessionName(session[kType])}: goaway ${code} ` +
508-
`received [last stream id: ${lastStreamID}]`);
523+
debugSessionObj(session, 'goaway %d received [last stream id: %d]',
524+
code, lastStreamID);
509525

510526
const state = session[kState];
511527
state.goawayCode = code;
@@ -560,8 +576,7 @@ function requestOnConnect(headers, options) {
560576
return;
561577
}
562578

563-
debug(`Http2Session ${sessionName(session[kType])}: connected, ` +
564-
'initializing request');
579+
debugSessionObj(session, 'connected, initializing request');
565580

566581
let streamOptions = 0;
567582
if (options.endStream)
@@ -650,13 +665,13 @@ function settingsCallback(cb, ack, duration) {
650665
this[kState].pendingAck--;
651666
this[kLocalSettings] = undefined;
652667
if (ack) {
653-
debug(`Http2Session ${sessionName(this[kType])}: settings received`);
668+
debugSessionObj(this, 'settings received');
654669
const settings = this.localSettings;
655670
if (typeof cb === 'function')
656671
cb(null, settings, duration);
657672
this.emit('localSettings', settings);
658673
} else {
659-
debug(`Http2Session ${sessionName(this[kType])}: settings canceled`);
674+
debugSessionObj(this, 'settings canceled');
660675
if (typeof cb === 'function')
661676
cb(new ERR_HTTP2_SETTINGS_CANCEL());
662677
}
@@ -666,7 +681,7 @@ function settingsCallback(cb, ack, duration) {
666681
function submitSettings(settings, callback) {
667682
if (this.destroyed)
668683
return;
669-
debug(`Http2Session ${sessionName(this[kType])}: submitting settings`);
684+
debugSessionObj(this, 'submitting settings');
670685
this[kUpdateTimer]();
671686
updateSettingsBuffer(settings);
672687
if (!this[kHandle].settings(settingsCallback.bind(this, callback))) {
@@ -700,7 +715,7 @@ function submitPriority(options) {
700715
function submitGoaway(code, lastStreamID, opaqueData) {
701716
if (this.destroyed)
702717
return;
703-
debug(`Http2Session ${sessionName(this[kType])}: submitting goaway`);
718+
debugSessionObj(this, 'submitting goaway');
704719
this[kUpdateTimer]();
705720
this[kHandle].goaway(code, lastStreamID, opaqueData);
706721
}
@@ -831,7 +846,7 @@ function setupHandle(socket, type, options) {
831846
'Internal HTTP/2 Failure. The socket is not connected. Please ' +
832847
'report this as a bug in Node.js');
833848

834-
debug(`Http2Session ${sessionName(type)}: setting up session handle`);
849+
debugSession(type, 'setting up session handle');
835850
this[kState].flags |= SESSION_FLAGS_READY;
836851

837852
updateOptionsBuffer(options);
@@ -987,7 +1002,7 @@ class Http2Session extends EventEmitter {
9871002
setupFn();
9881003
}
9891004

990-
debug(`Http2Session ${sessionName(type)}: created`);
1005+
debugSession(type, 'created');
9911006
}
9921007

9931008
// Returns undefined if the socket is not yet connected, true if the
@@ -1163,7 +1178,7 @@ class Http2Session extends EventEmitter {
11631178

11641179
if (callback && typeof callback !== 'function')
11651180
throw new ERR_INVALID_CALLBACK(callback);
1166-
debug(`Http2Session ${sessionName(this[kType])}: sending settings`);
1181+
debugSessionObj(this, 'sending settings');
11671182

11681183
this[kState].pendingAck++;
11691184

@@ -1204,7 +1219,7 @@ class Http2Session extends EventEmitter {
12041219
destroy(error = NGHTTP2_NO_ERROR, code) {
12051220
if (this.destroyed)
12061221
return;
1207-
debug(`Http2Session ${sessionName(this[kType])}: destroying`);
1222+
debugSessionObj(this, 'destroying');
12081223

12091224
if (typeof error === 'number') {
12101225
code = error;
@@ -1261,7 +1276,7 @@ class Http2Session extends EventEmitter {
12611276
close(callback) {
12621277
if (this.closed || this.destroyed)
12631278
return;
1264-
debug(`Http2Session ${sessionName(this[kType])}: marking session closed`);
1279+
debugSessionObj(this, 'marking session closed');
12651280
this[kState].flags |= SESSION_FLAGS_CLOSED;
12661281
if (typeof callback === 'function')
12671282
this.once('close', callback);
@@ -1432,7 +1447,7 @@ class ClientHttp2Session extends Http2Session {
14321447
// Submits a new HTTP2 request to the connected peer. Returns the
14331448
// associated Http2Stream instance.
14341449
request(headers, options) {
1435-
debug(`Http2Session ${sessionName(this[kType])}: initiating request`);
1450+
debugSessionObj(this, 'initiating request');
14361451

14371452
if (this.destroyed)
14381453
throw new ERR_HTTP2_INVALID_SESSION();
@@ -1832,8 +1847,7 @@ class Http2Stream extends Duplex {
18321847
if (this.pending) {
18331848
this.once('ready', () => this._final(cb));
18341849
} else if (handle !== undefined) {
1835-
debug(`Http2Stream ${this[kID]} [Http2Session ` +
1836-
`${sessionName(this[kSession][kType])}]: _final shutting down`);
1850+
debugStreamObj(this, '_final shutting down');
18371851
const req = new ShutdownWrap();
18381852
req.oncomplete = afterShutdown;
18391853
req.callback = cb;
@@ -1892,9 +1906,7 @@ class Http2Stream extends Duplex {
18921906
assertIsObject(headers, 'headers');
18931907
headers = Object.assign(Object.create(null), headers);
18941908

1895-
const session = this[kSession];
1896-
debug(`Http2Stream ${this[kID]} [Http2Session ` +
1897-
`${sessionName(session[kType])}]: sending trailers`);
1909+
debugStreamObj(this, 'sending trailers');
18981910

18991911
this[kUpdateTimer]();
19001912

@@ -1947,8 +1959,7 @@ class Http2Stream extends Duplex {
19471959
const handle = this[kHandle];
19481960
const id = this[kID];
19491961

1950-
debug(`Http2Stream ${this[kID] || '<pending>'} [Http2Session ` +
1951-
`${sessionName(session[kType])}]: destroying stream`);
1962+
debugStream(this[kID] || 'pending', session[kType], 'destroying stream');
19521963

19531964
const state = this[kState];
19541965
const sessionCode = session[kState].goawayCode ||
@@ -2274,8 +2285,7 @@ class ServerHttp2Stream extends Http2Stream {
22742285

22752286
const session = this[kSession];
22762287

2277-
debug(`Http2Stream ${this[kID]} [Http2Session ` +
2278-
`${sessionName(session[kType])}]: initiating push stream`);
2288+
debugStreamObj(this, 'initiating push stream');
22792289

22802290
this[kUpdateTimer]();
22812291

@@ -2355,9 +2365,7 @@ class ServerHttp2Stream extends Http2Stream {
23552365
assertIsObject(options, 'options');
23562366
options = { ...options };
23572367

2358-
const session = this[kSession];
2359-
debug(`Http2Stream ${this[kID]} [Http2Session ` +
2360-
`${sessionName(session[kType])}]: initiating response`);
2368+
debugStreamObj(this, 'initiating response');
23612369
this[kUpdateTimer]();
23622370

23632371
options.endStream = !!options.endStream;
@@ -2430,8 +2438,7 @@ class ServerHttp2Stream extends Http2Stream {
24302438

24312439
validateNumber(fd, 'fd');
24322440

2433-
debug(`Http2Stream ${this[kID]} [Http2Session ` +
2434-
`${sessionName(session[kType])}]: initiating response from fd`);
2441+
debugStreamObj(this, 'initiating response from fd');
24352442
this[kUpdateTimer]();
24362443
this.ownsFd = false;
24372444

@@ -2492,8 +2499,7 @@ class ServerHttp2Stream extends Http2Stream {
24922499
}
24932500

24942501
const session = this[kSession];
2495-
debug(`Http2Stream ${this[kID]} [Http2Session ` +
2496-
`${sessionName(session[kType])}]: initiating response from file`);
2502+
debugStreamObj(this, 'initiating response from file');
24972503
this[kUpdateTimer]();
24982504
this.ownsFd = true;
24992505

@@ -2527,9 +2533,7 @@ class ServerHttp2Stream extends Http2Stream {
25272533
assertIsObject(headers, 'headers');
25282534
headers = Object.assign(Object.create(null), headers);
25292535

2530-
const session = this[kSession];
2531-
debug(`Http2Stream ${this[kID]} [Http2Session ` +
2532-
`${sessionName(session[kType])}]: sending additional headers`);
2536+
debugStreamObj(this, 'sending additional headers');
25332537

25342538
if (headers[HTTP2_HEADER_STATUS] != null) {
25352539
const statusCode = headers[HTTP2_HEADER_STATUS] |= 0;
@@ -2590,8 +2594,7 @@ function socketOnError(error) {
25902594
// we can do and the other side is fully within its rights to do so.
25912595
if (error.code === 'ECONNRESET' && session[kState].goawayCode !== null)
25922596
return session.destroy();
2593-
debug(`Http2Session ${sessionName(session[kType])}: socket error [` +
2594-
`${error.message}]`);
2597+
debugSessionObj(this, 'socket error [%s]', error.message);
25952598
session.destroy(error);
25962599
}
25972600
}
@@ -2636,7 +2639,8 @@ function connectionListener(socket) {
26362639
return httpConnectionListener.call(this, socket);
26372640
}
26382641
// Let event handler deal with the socket
2639-
debug(`Unknown protocol from ${socket.remoteAddress}:${socket.remotePort}`);
2642+
debug('Unknown protocol from %s:%s',
2643+
socket.remoteAddress, socket.remotePort);
26402644
if (!this.emit('unknownProtocol', socket)) {
26412645
// We don't know what to do, so let's just tell the other side what's
26422646
// going on in a format that they *might* understand.
@@ -2761,7 +2765,7 @@ function setupCompat(ev) {
27612765
function socketOnClose() {
27622766
const session = this[kSession];
27632767
if (session !== undefined) {
2764-
debug(`Http2Session ${sessionName(session[kType])}: socket closed`);
2768+
debugSessionObj(session, 'socket closed');
27652769
const err = session.connecting ? new ERR_SOCKET_CLOSED() : null;
27662770
const state = session[kState];
27672771
state.streams.forEach((stream) => stream.close(NGHTTP2_CANCEL));

0 commit comments

Comments
 (0)