Skip to content

Commit a77d94f

Browse files
committed
Based on #2139 I wrapped http2session.ping in a try-catch block again
1 parent d325b5f commit a77d94f

File tree

2 files changed

+78
-53
lines changed

2 files changed

+78
-53
lines changed

packages/grpc-js/src/server.ts

Lines changed: 57 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,29 +1467,35 @@ export class Server {
14671467
this.keepaliveTrace(
14681468
'Sending ping with timeout ' + this.keepaliveTimeoutMs + 'ms'
14691469
);
1470-
const pingSentSuccessfully = session.ping(
1471-
(err: Error | null, duration: number, payload: Buffer) => {
1472-
clearKeepaliveTimeout();
1473-
if (err) {
1474-
this.keepaliveTrace('Ping failed with error: ' + err.message);
1475-
this.trace(
1476-
'Connection dropped due to error of a ping frame ' +
1477-
err.message +
1478-
' return in ' +
1479-
duration
1480-
);
1481-
sessionClosedByServer = true;
1482-
session.close();
1483-
} else {
1484-
this.keepaliveTrace('Received ping response');
1485-
maybeStartKeepalivePingTimer();
1470+
let pingSendError = '';
1471+
try {
1472+
const pingSentSuccessfully = session.ping(
1473+
(err: Error | null, duration: number, payload: Buffer) => {
1474+
clearKeepaliveTimeout();
1475+
if (err) {
1476+
this.keepaliveTrace('Ping failed with error: ' + err.message);
1477+
sessionClosedByServer = true;
1478+
session.close();
1479+
} else {
1480+
this.keepaliveTrace('Received ping response');
1481+
maybeStartKeepalivePingTimer();
1482+
}
14861483
}
1484+
);
1485+
if (!pingSentSuccessfully) {
1486+
pingSendError = 'Ping returned false';
14871487
}
1488-
);
1488+
} catch (e) {
1489+
// grpc/grpc-node#2139
1490+
pingSendError =
1491+
(e instanceof Error ? e.message : '') || 'Unknown error';
1492+
}
14891493

1490-
if (!pingSentSuccessfully) {
1491-
this.keepaliveTrace('Ping failed to send');
1492-
this.trace('Connection dropped due to failure to send ping frame');
1494+
if (pingSendError) {
1495+
this.keepaliveTrace('Ping send failed: ' + pingSendError);
1496+
this.trace(
1497+
'Connection dropped due to ping send error: ' + pingSendError
1498+
);
14931499
sessionClosedByServer = true;
14941500
session.close();
14951501
return;
@@ -1650,32 +1656,42 @@ export class Server {
16501656
this.keepaliveTrace(
16511657
'Sending ping with timeout ' + this.keepaliveTimeoutMs + 'ms'
16521658
);
1653-
const pingSentSuccessfully = session.ping(
1654-
(err: Error | null, duration: number, payload: Buffer) => {
1655-
clearKeepaliveTimeout();
1656-
if (err) {
1657-
this.keepaliveTrace('Ping failed with error: ' + err.message);
1658-
this.channelzTrace.addTrace(
1659-
'CT_INFO',
1660-
'Connection dropped due to error of a ping frame ' +
1661-
err.message +
1662-
' return in ' +
1663-
duration
1664-
);
1665-
sessionClosedByServer = true;
1666-
session.close();
1667-
} else {
1668-
this.keepaliveTrace('Received ping response');
1669-
maybeStartKeepalivePingTimer();
1659+
let pingSendError = '';
1660+
try {
1661+
const pingSentSuccessfully = session.ping(
1662+
(err: Error | null, duration: number, payload: Buffer) => {
1663+
clearKeepaliveTimeout();
1664+
if (err) {
1665+
this.keepaliveTrace('Ping failed with error: ' + err.message);
1666+
this.channelzTrace.addTrace(
1667+
'CT_INFO',
1668+
'Connection dropped due to error of a ping frame ' +
1669+
err.message +
1670+
' return in ' +
1671+
duration
1672+
);
1673+
sessionClosedByServer = true;
1674+
session.close();
1675+
} else {
1676+
this.keepaliveTrace('Received ping response');
1677+
maybeStartKeepalivePingTimer();
1678+
}
16701679
}
1680+
);
1681+
if (!pingSentSuccessfully) {
1682+
pingSendError = 'Ping returned false';
16711683
}
1672-
);
1684+
} catch (e) {
1685+
// grpc/grpc-node#2139
1686+
pingSendError =
1687+
(e instanceof Error ? e.message : '') || 'Unknown error';
1688+
}
16731689

1674-
if (!pingSentSuccessfully) {
1675-
this.keepaliveTrace('Ping failed to send');
1690+
if (pingSendError) {
1691+
this.keepaliveTrace('Ping send failed: ' + pingSendError);
16761692
this.channelzTrace.addTrace(
16771693
'CT_INFO',
1678-
'Connection dropped due failure to send ping frame'
1694+
'Connection dropped due to ping send error: ' + pingSendError
16791695
);
16801696
sessionClosedByServer = true;
16811697
session.close();

packages/grpc-js/src/transport.ts

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -431,20 +431,29 @@ class Http2Transport implements Transport {
431431
this.handleDisconnect();
432432
}, this.keepaliveTimeoutMs);
433433
this.keepaliveTimeout.unref?.();
434-
const pingSentSuccessfully = this.session.ping(
435-
(err: Error | null, duration: number, payload: Buffer) => {
436-
this.clearKeepaliveTimeout();
437-
if (err) {
438-
this.keepaliveTrace('Ping failed with error ' + err.message);
439-
this.handleDisconnect();
440-
} else {
441-
this.keepaliveTrace('Received ping response');
442-
this.maybeStartKeepalivePingTimer();
434+
let pingSendError = '';
435+
try {
436+
const pingSentSuccessfully = this.session.ping(
437+
(err: Error | null, duration: number, payload: Buffer) => {
438+
this.clearKeepaliveTimeout();
439+
if (err) {
440+
this.keepaliveTrace('Ping failed with error ' + err.message);
441+
this.handleDisconnect();
442+
} else {
443+
this.keepaliveTrace('Received ping response');
444+
this.maybeStartKeepalivePingTimer();
445+
}
443446
}
447+
);
448+
if (!pingSentSuccessfully) {
449+
pingSendError = 'Ping returned false';
444450
}
445-
);
446-
if (!pingSentSuccessfully) {
447-
this.keepaliveTrace('Ping failed to send');
451+
} catch (e) {
452+
// grpc/grpc-node#2139
453+
pingSendError = (e instanceof Error ? e.message : '') || 'Unknown error';
454+
}
455+
if (pingSendError) {
456+
this.keepaliveTrace('Ping send failed: ' + pingSendError);
448457
this.handleDisconnect();
449458
}
450459
}

0 commit comments

Comments
 (0)