From ddec63af2055038ebbe235e73b2371cdd2cfaac0 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 4 Aug 2020 11:22:18 -0700 Subject: [PATCH 1/8] grpc-js: Clean up call even if status throws an error --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/call-stream.ts | 11 ++++++++++- packages/grpc-js/src/server-call.ts | 6 ++++++ packages/grpc-js/src/server.ts | 12 +++++++++++- packages/grpc-js/src/subchannel.ts | 2 +- 5 files changed, 29 insertions(+), 4 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 4c4e59f0..70ebe057 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.1.3", + "version": "1.1.4", "description": "gRPC Library for Node - pure JS implementation", "homepage": "https://grpc.io/", "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js", diff --git a/packages/grpc-js/src/call-stream.ts b/packages/grpc-js/src/call-stream.ts index 825342a5..45bc665f 100644 --- a/packages/grpc-js/src/call-stream.ts +++ b/packages/grpc-js/src/call-stream.ts @@ -227,7 +227,15 @@ export class Http2CallStream implements Call { const filteredStatus = this.filterStack.receiveTrailers( this.finalStatus! ); - this.listener?.onReceiveStatus(filteredStatus); + /* We delay the actual action of bubbling up the status to insulate the + * cleanup code in this class from any errors that may be thrown in the + * upper layers as a result of bubbling up the status. In particular, + * if the status is not OK, the "error" event may be emitted + * synchronously at the top level, which will result in a thrown error if + * the user does not handle that event. */ + process.nextTick(() => { + this.listener?.onReceiveStatus(filteredStatus); + }); if (this.subchannel) { this.subchannel.callUnref(); this.subchannel.removeDisconnectListener(this.disconnectListener); @@ -602,6 +610,7 @@ export class Http2CallStream implements Call { } else { code = http2.constants.NGHTTP2_CANCEL; } + this.trace('close http2 stream with code ' + code); this.http2Stream.close(code); } } diff --git a/packages/grpc-js/src/server-call.ts b/packages/grpc-js/src/server-call.ts index 82d88517..fb76ca03 100644 --- a/packages/grpc-js/src/server-call.ts +++ b/packages/grpc-js/src/server-call.ts @@ -373,6 +373,12 @@ export class Http2ServerCallStream< }); this.stream.once('close', () => { + trace( + 'Request to method ' + + this.handler?.path + + ' stream closed with rstCode ' + + this.stream.rstCode + ); this.cancelled = true; this.emit('cancelled', 'cancelled'); }); diff --git a/packages/grpc-js/src/server.ts b/packages/grpc-js/src/server.ts index ebd4504d..3580d89a 100644 --- a/packages/grpc-js/src/server.ts +++ b/packages/grpc-js/src/server.ts @@ -543,11 +543,21 @@ export class Server { try { const path = headers[http2.constants.HTTP2_HEADER_PATH] as string; + const serverAddress = http2Server.address(); + let serverAddressString = 'null'; + if (serverAddress) { + if (typeof serverAddress === 'string') { + serverAddressString = serverAddress; + } else { + serverAddressString = + serverAddress.address + ':' + serverAddress.port; + } + } trace( 'Received call to method ' + path + ' at address ' + - http2Server.address()?.toString() + serverAddressString ); const handler = this.handlers.get(path); diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index be72680e..736562a6 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -415,7 +415,7 @@ export class Subchannel { ); } trace( - this.subchannelAddress + + this.subchannelAddressString + ' connection closed by GOAWAY with code ' + errorCode ); From d9b3f9f3645c57b2168f92af1994e37f5973d1e5 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 4 Aug 2020 11:37:08 -0700 Subject: [PATCH 2/8] grpc-js: Add end(md?: Metadata) method to streaming server calls --- packages/grpc-js/src/server-call.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js/src/server-call.ts b/packages/grpc-js/src/server-call.ts index 82d88517..1e935995 100644 --- a/packages/grpc-js/src/server-call.ts +++ b/packages/grpc-js/src/server-call.ts @@ -91,10 +91,13 @@ export type ServerWritableStream< RequestType, ResponseType > = ServerSurfaceCall & - ObjectWritable & { request: RequestType | null }; + ObjectWritable & { + request: RequestType | null; + end: (metadata?: Metadata) => void; + }; export type ServerDuplexStream = ServerSurfaceCall & ObjectReadable & - ObjectWritable; + ObjectWritable & { end: (metadata?: Metadata) => void }; export class ServerUnaryCallImpl extends EventEmitter implements ServerUnaryCall { @@ -255,6 +258,15 @@ export class ServerDuplexStreamImpl extends Duplex sendMetadata(responseMetadata: Metadata): void { this.call.sendMetadata(responseMetadata); } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + end(metadata?: any) { + if (metadata) { + this.trailingMetadata = metadata; + } + + super.end(); + } } ServerDuplexStreamImpl.prototype._read = From 33a4c85f89591c36cb7001d604ce1f62b65e2db6 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 4 Aug 2020 13:03:08 -0700 Subject: [PATCH 3/8] grpc-js: Implement getPeer on the client and server --- packages/grpc-js/src/call-stream.ts | 2 +- packages/grpc-js/src/call.ts | 8 ++++---- packages/grpc-js/src/server-call.ts | 21 +++++++++++++++++---- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/grpc-js/src/call-stream.ts b/packages/grpc-js/src/call-stream.ts index 825342a5..2c9f01cb 100644 --- a/packages/grpc-js/src/call-stream.ts +++ b/packages/grpc-js/src/call-stream.ts @@ -630,7 +630,7 @@ export class Http2CallStream implements Call { } getPeer(): string { - throw new Error('Not yet implemented'); + return this.subchannel?.getAddress() ?? this.channel.getTarget(); } getMethod(): string { diff --git a/packages/grpc-js/src/call.ts b/packages/grpc-js/src/call.ts index 0d88ef15..cfe37ecf 100644 --- a/packages/grpc-js/src/call.ts +++ b/packages/grpc-js/src/call.ts @@ -93,7 +93,7 @@ export class ClientUnaryCallImpl extends EventEmitter } getPeer(): string { - return this.call?.getPeer() ?? ''; + return this.call?.getPeer() ?? 'unknown'; } } @@ -109,7 +109,7 @@ export class ClientReadableStreamImpl extends Readable } getPeer(): string { - return this.call?.getPeer() ?? ''; + return this.call?.getPeer() ?? 'unknown'; } _read(_size: number): void { @@ -129,7 +129,7 @@ export class ClientWritableStreamImpl extends Writable } getPeer(): string { - return this.call?.getPeer() ?? ''; + return this.call?.getPeer() ?? 'unknown'; } _write(chunk: RequestType, encoding: string, cb: WriteCallback) { @@ -164,7 +164,7 @@ export class ClientDuplexStreamImpl extends Duplex } getPeer(): string { - return this.call?.getPeer() ?? ''; + return this.call?.getPeer() ?? 'unknown'; } _read(_size: number): void { diff --git a/packages/grpc-js/src/server-call.ts b/packages/grpc-js/src/server-call.ts index 82d88517..bd992773 100644 --- a/packages/grpc-js/src/server-call.ts +++ b/packages/grpc-js/src/server-call.ts @@ -112,7 +112,7 @@ export class ServerUnaryCallImpl extends EventEmitter } getPeer(): string { - throw new Error('not implemented yet'); + return this.call.getPeer(); } sendMetadata(responseMetadata: Metadata): void { @@ -145,7 +145,7 @@ export class ServerReadableStreamImpl } getPeer(): string { - throw new Error('not implemented yet'); + return this.call.getPeer(); } sendMetadata(responseMetadata: Metadata): void { @@ -178,7 +178,7 @@ export class ServerWritableStreamImpl } getPeer(): string { - throw new Error('not implemented yet'); + return this.call.getPeer(); } sendMetadata(responseMetadata: Metadata): void { @@ -249,7 +249,7 @@ export class ServerDuplexStreamImpl extends Duplex } getPeer(): string { - throw new Error('not implemented yet'); + return this.call.getPeer(); } sendMetadata(responseMetadata: Metadata): void { @@ -738,6 +738,19 @@ export class Http2ServerCallStream< ); } } + + getPeer(): string { + const socket = this.stream.session.socket; + if (socket.remoteAddress) { + if (socket.remotePort) { + return `${socket.remoteAddress}:${socket.remotePort}`; + } else { + return socket.remoteAddress; + } + } else { + return 'unknown'; + } + } } /* eslint-disable @typescript-eslint/no-explicit-any */ From f0ed1aba14ff53e356343178d5634af5e624a3f6 Mon Sep 17 00:00:00 2001 From: Martin <1224973+mavaa@users.noreply.github.com> Date: Wed, 12 Aug 2020 16:51:42 +0200 Subject: [PATCH 4/8] Fix incorrectly named grpc-tools flag Was "--generate_package_definitions" (with an s) but should be "generate_package_definition" as in the documentation here: https://github.com/grpc/grpc-node/tree/master/packages/grpc-tools --- packages/grpc-js/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/README.md b/packages/grpc-js/README.md index 4d0cb548..dadcd188 100644 --- a/packages/grpc-js/README.md +++ b/packages/grpc-js/README.md @@ -28,7 +28,7 @@ This library does not directly handle `.proto` files. To use `.proto` files with `@grpc/grpc-js` is almost a drop-in replacement for `grpc`, but you may need to make a few code changes to use it: - If you are currently loading `.proto` files using `grpc.load`, that function is not available in this library. You should instead load your `.proto` files using `@grpc/proto-loader` and load the resulting package definition objects into `@grpc/grpc-js` using `grpc.loadPackageDefinition`. -- If you are currently loading packages generated by `grpc-tools`, you should instead generate your files using the `--generate_package_definitions` option in `grpc-tools`, then load the object exported by the generated file into `@grpc/grpc-js` using `grpc.loadPackageDefinition`. +- If you are currently loading packages generated by `grpc-tools`, you should instead generate your files using the `generate_package_definition` option in `grpc-tools`, then load the object exported by the generated file into `@grpc/grpc-js` using `grpc.loadPackageDefinition`. - If you have a server and you are using `Server#bind` to bind ports, you will need to use `Server#bindAsync` instead. ## Some Notes on API Guarantees From 1583786478042de1dd61300e810bc87d442d6c88 Mon Sep 17 00:00:00 2001 From: Thomas Hunter II Date: Sun, 16 Aug 2020 13:50:23 -0700 Subject: [PATCH 5/8] Add link to grpc docs in @grpc/grpc-js README - Adds a link to `grpc` documentation - Addresses some of the concerns in #1540 --- packages/grpc-js/README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/grpc-js/README.md b/packages/grpc-js/README.md index dadcd188..e34cee9c 100644 --- a/packages/grpc-js/README.md +++ b/packages/grpc-js/README.md @@ -8,6 +8,10 @@ Node 12 is recommended. The exact set of compatible Node versions can be found i npm install @grpc/grpc-js ``` +## Documentation + +Documentation specifically for the `@grpc/grpc-js` package is currently not available. However, [documentation is available for the `grpc` package](https://grpc.github.io/grpc/node/grpc.html), and the two packages contain mostly the same interface. There are a few notable differences, however, and these differences are noted in the "Migrating from grpc" section below. + ## Features - Clients From 74930526724826892bb7696412bd6e7c2c2b4ced Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 18 Aug 2020 15:13:12 -0700 Subject: [PATCH 6/8] grpc-js: Move @types/node to a production dependency --- packages/grpc-js/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index ee910cfb..839c67f3 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -21,7 +21,6 @@ "@types/lodash": "^4.14.108", "@types/mocha": "^5.2.6", "@types/ncp": "^2.0.1", - "@types/node": "^12.7.5", "@types/pify": "^3.0.2", "@types/semver": "^6.0.1", "clang-format": "^1.0.55", @@ -58,6 +57,7 @@ "posttest": "npm run check" }, "dependencies": { + "@types/node": "^12.12.47", "semver": "^6.2.0" }, "files": [ From 917b4fca77fc9ee81c8dc71d38043f54c4d98562 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 19 Aug 2020 10:24:20 -0400 Subject: [PATCH 7/8] Prevent mutation of default headers --- packages/grpc-js/src/server-call.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/src/server-call.ts b/packages/grpc-js/src/server-call.ts index 82d88517..bdefa68a 100644 --- a/packages/grpc-js/src/server-call.ts +++ b/packages/grpc-js/src/server-call.ts @@ -413,7 +413,7 @@ export class Http2ServerCallStream< this.metadataSent = true; const custom = customMetadata ? customMetadata.toHttp2Headers() : null; // TODO(cjihrig): Include compression headers. - const headers = Object.assign(defaultResponseHeaders, custom); + const headers = Object.assign({}, defaultResponseHeaders, custom); this.stream.respond(headers, defaultResponseOptions); } From 5abb47390fa34078f5afd21da23bf488d79c0840 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 20 Aug 2020 09:24:57 -0700 Subject: [PATCH 8/8] grpc-js: Move a couple of dev dependencies to prod --- packages/grpc-js/package.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index f207c753..7deb248e 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.1.4", + "version": "1.1.5", "description": "gRPC Library for Node - pure JS implementation", "homepage": "https://grpc.io/", "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js", @@ -15,7 +15,6 @@ "types": "build/src/index.d.ts", "license": "Apache-2.0", "devDependencies": { - "@grpc/proto-loader": "^0.6.0-pre6", "@types/gulp": "^4.0.6", "@types/gulp-mocha": "0.0.32", "@types/lodash": "^4.14.108", @@ -25,7 +24,6 @@ "@types/semver": "^6.0.1", "clang-format": "^1.0.55", "execa": "^2.0.3", - "google-auth-library": "^6.0.0", "gts": "^2.0.0", "gulp": "^4.0.2", "gulp-mocha": "^6.0.0", @@ -57,7 +55,9 @@ "posttest": "npm run check" }, "dependencies": { + "@grpc/proto-loader": "^0.6.0-pre14", "@types/node": "^12.12.47", + "google-auth-library": "^6.0.0", "semver": "^6.2.0" }, "files": [