Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

grpc-js: Speculative fix for ECONNRESET errors #1705

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/grpc-js/package.json
@@ -1,6 +1,6 @@
{
"name": "@grpc/grpc-js",
"version": "1.2.8",
"version": "1.2.9",
"description": "gRPC Library for Node - pure JS implementation",
"homepage": "https://grpc.io/",
"repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",
Expand Down
53 changes: 43 additions & 10 deletions packages/grpc-js/src/call-stream.ts
Expand Up @@ -16,6 +16,7 @@
*/

import * as http2 from 'http2';
import * as os from 'os';

import { CallCredentials } from './call-credentials';
import { Propagate, Status } from './constants';
Expand All @@ -37,8 +38,34 @@ const {
NGHTTP2_CANCEL,
} = http2.constants;

interface NodeError extends Error {
/**
* https://nodejs.org/api/errors.html#errors_class_systemerror
*/
interface SystemError extends Error {
address?: string;
code: string;
dest?: string;
errno: number;
info?: object;
message: string;
path?: string;
port?: number;
syscall: string;
}

/**
* Should do approximately the same thing as util.getSystemErrorName but the
* TypeScript types don't have that function for some reason so I just made my
* own.
* @param errno
*/
function getSystemErrorName(errno: number): string {
for (const [name, num] of Object.entries(os.constants.errno)) {
if (num === errno) {
return name;
}
}
return 'Unknown system error ' + errno;
}

export type Deadline = Date | number;
Expand Down Expand Up @@ -206,7 +233,7 @@ export class Http2CallStream implements Call {

private listener: InterceptingListener | null = null;

private internalErrorMessage: string | null = null;
private internalError: SystemError | null = null;

constructor(
private readonly methodName: string,
Expand Down Expand Up @@ -567,19 +594,24 @@ export class Http2CallStream implements Call {
break;
case http2.constants.NGHTTP2_INTERNAL_ERROR:
code = Status.INTERNAL;
if (this.internalErrorMessage === null) {
if (this.internalError === null) {
/* This error code was previously handled in the default case, and
* there are several instances of it online, so I wanted to
* preserve the original error message so that people find existing
* information in searches, but also include the more recognizable
* "Internal server error" message. */
details = `Received RST_STREAM with code ${stream.rstCode} (Internal server error)`;
} else {
/* The "Received RST_STREAM with code ..." error is preserved
* here for continuity with errors reported online, but the
* error message at the end will probably be more relevant in
* most cases. */
details = `Received RST_STREAM with code ${stream.rstCode} triggered by internal client error: ${this.internalErrorMessage}`;
if (this.internalError.errno === os.constants.errno.ECONNRESET) {
code = Status.UNAVAILABLE;
details = this.internalError.message;
} else {
/* The "Received RST_STREAM with code ..." error is preserved
* here for continuity with errors reported online, but the
* error message at the end will probably be more relevant in
* most cases. */
details = `Received RST_STREAM with code ${stream.rstCode} triggered by internal client error: ${this.internalError.message}`;
}
}
break;
default:
Expand All @@ -593,7 +625,7 @@ export class Http2CallStream implements Call {
this.endCall({ code, details, metadata: new Metadata() });
});
});
stream.on('error', (err: NodeError) => {
stream.on('error', (err: SystemError) => {
/* We need an error handler here to stop "Uncaught Error" exceptions
* from bubbling up. However, errors here should all correspond to
* "close" events, where we will handle the error more granularly */
Expand All @@ -602,7 +634,8 @@ export class Http2CallStream implements Call {
* https://github.com/nodejs/node/blob/8b8620d580314050175983402dfddf2674e8e22a/lib/internal/http2/core.js#L2267
*/
if (err.code !== 'ERR_HTTP2_STREAM_ERROR') {
this.internalErrorMessage = err.message;
this.trace('Node error event: message=' + err.message + ' code=' + err.code + ' errno=' + getSystemErrorName(err.errno) + ' syscall=' + err.syscall);
this.internalError = err;
}
});
if (!this.pendingRead) {
Expand Down