From 58206feb3f68b5ef24f48fd9d8a7c6de79538127 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Thu, 7 Dec 2017 14:20:24 -0800 Subject: [PATCH 1/3] grpc-js-core: compiler error fixes --- packages/grpc-js-core/src/call-credentials.ts | 10 +++++----- packages/grpc-js-core/src/metadata.ts | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/grpc-js-core/src/call-credentials.ts b/packages/grpc-js-core/src/call-credentials.ts index 5b322402f..3be454d5f 100644 --- a/packages/grpc-js-core/src/call-credentials.ts +++ b/packages/grpc-js-core/src/call-credentials.ts @@ -3,7 +3,7 @@ import {map, reduce} from 'lodash'; import {Metadata} from './metadata'; export type CallMetadataGenerator = - (options: Object, cb: (err: Error|null, metadata?: Metadata) => void) => + (options: {}, cb: (err: Error|null, metadata?: Metadata) => void) => void; /** @@ -15,7 +15,7 @@ export interface CallCredentials { * Asynchronously generates a new Metadata object. * @param options Options used in generating the Metadata object. */ - generateMetadata(options: Object): Promise; + generateMetadata(options: {}): Promise; /** * Creates a new CallCredentials object from properties of both this and * another CallCredentials object. This object's metadata generator will be @@ -28,7 +28,7 @@ export interface CallCredentials { class ComposedCallCredentials implements CallCredentials { constructor(private creds: CallCredentials[]) {} - async generateMetadata(options: Object): Promise { + async generateMetadata(options: {}): Promise { let base: Metadata = new Metadata(); let generated: Metadata[] = await Promise.all( map(this.creds, (cred) => cred.generateMetadata(options))); @@ -46,7 +46,7 @@ class ComposedCallCredentials implements CallCredentials { class SingleCallCredentials implements CallCredentials { constructor(private metadataGenerator: CallMetadataGenerator) {} - async generateMetadata(options: Object): Promise { + async generateMetadata(options: {}): Promise { return new Promise((resolve, reject) => { this.metadataGenerator(options, (err, metadata) => { if (metadata !== undefined) { @@ -64,7 +64,7 @@ class SingleCallCredentials implements CallCredentials { } class EmptyCallCredentials implements CallCredentials { - async generateMetadata(options: Object): Promise { + async generateMetadata(options: {}): Promise { return new Metadata(); } diff --git a/packages/grpc-js-core/src/metadata.ts b/packages/grpc-js-core/src/metadata.ts index c4bb2568e..9ed69dfcc 100644 --- a/packages/grpc-js-core/src/metadata.ts +++ b/packages/grpc-js-core/src/metadata.ts @@ -194,7 +194,7 @@ export class Metadata { values.forEach((value) => { result.add(key, Buffer.from(value, 'base64')); }); - } else { + } else if (values !== undefined) { result.add(key, Buffer.from(values, 'base64')); } } else { @@ -202,7 +202,7 @@ export class Metadata { values.forEach((value) => { result.add(key, value); }); - } else { + } else if (values !== undefined) { result.add(key, values); } } From 09cb531f9beb0001b202623b8462d7a37c346564 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Thu, 7 Dec 2017 10:38:32 -0800 Subject: [PATCH 2/3] grpc-js-core: prepend protocol before creating URL object from address --- packages/grpc-js-core/src/channel.ts | 13 +++++++------ packages/grpc-js-core/src/client.ts | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/grpc-js-core/src/channel.ts b/packages/grpc-js-core/src/channel.ts index a51ae9572..d28d8d592 100644 --- a/packages/grpc-js-core/src/channel.ts +++ b/packages/grpc-js-core/src/channel.ts @@ -71,6 +71,7 @@ export interface Channel extends EventEmitter { } export class Http2Channel extends EventEmitter implements Channel { + private readonly authority: url.URL; private connectivityState: ConnectivityState = ConnectivityState.IDLE; /* For now, we have up to one subchannel, which will exist as long as we are * connecting or trying to connect */ @@ -134,9 +135,9 @@ export class Http2Channel extends EventEmitter implements Channel { let subChannel: http2.ClientHttp2Session; let secureContext = this.credentials.getSecureContext(); if (secureContext === null) { - subChannel = http2.connect(this.address); + subChannel = http2.connect(this.authority); } else { - subChannel = http2.connect(this.address, {secureContext}); + subChannel = http2.connect(this.authority, {secureContext}); } this.subChannel = subChannel; let now = new Date(); @@ -165,14 +166,14 @@ export class Http2Channel extends EventEmitter implements Channel { } constructor( - private readonly address: url.URL, + address: string, public readonly credentials: ChannelCredentials, private readonly options: ChannelOptions) { super(); if (credentials.getSecureContext() === null) { - address.protocol = 'http'; + this.authority = new url.URL(`http://${address}`); } else { - address.protocol = 'https'; + this.authority = new url.URL(`https://${address}`); } this.filterStackFactory = new FilterStackFactory([ new CompressionFilterFactory(this), @@ -193,7 +194,7 @@ export class Http2Channel extends EventEmitter implements Channel { finalMetadata.then( (metadataValue) => { let headers = metadataValue.toHttp2Headers(); - headers[HTTP2_HEADER_AUTHORITY] = this.address.hostname; + headers[HTTP2_HEADER_AUTHORITY] = this.authority.hostname; headers[HTTP2_HEADER_CONTENT_TYPE] = 'application/grpc'; headers[HTTP2_HEADER_METHOD] = 'POST'; headers[HTTP2_HEADER_PATH] = methodName; diff --git a/packages/grpc-js-core/src/client.ts b/packages/grpc-js-core/src/client.ts index 59a84d24f..2b040e6be 100644 --- a/packages/grpc-js-core/src/client.ts +++ b/packages/grpc-js-core/src/client.ts @@ -24,7 +24,7 @@ export class Client { } // TODO(murgatroid99): Figure out how to get version number // options['grpc.primary_user_agent'] += 'grpc-node/' + version; - this.channel = new Http2Channel(new URL(address), credentials, options); + this.channel = new Http2Channel(address, credentials, options); } close(): void { From b7eb3d6988510df3c0881a2da2aa5fcc90eb4ff1 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Thu, 7 Dec 2017 13:49:58 -0800 Subject: [PATCH 3/3] grpc-js-core: various fixes --- packages/grpc-js-core/src/call-stream.ts | 16 +++++++++------- packages/grpc-js-core/src/metadata.ts | 1 + 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/grpc-js-core/src/call-stream.ts b/packages/grpc-js-core/src/call-stream.ts index 51c587568..f62e42afa 100644 --- a/packages/grpc-js-core/src/call-stream.ts +++ b/packages/grpc-js-core/src/call-stream.ts @@ -182,9 +182,10 @@ export class Http2CallStream extends Duplex implements CallStream { this.cancelWithStatus(Status.UNKNOWN, error.message); }); }); - stream.on('trailers', (headers) => { + stream.on('trailers', (headers: http2.IncomingHttpHeaders) => { let code: Status = this.mappedStatusCode; - if (headers.hasOwnProperty('grpc-status')) { + let details = ''; + if (typeof headers['grpc-status'] === 'string') { let receivedCode = Number(headers['grpc-status']); if (receivedCode in Status) { code = receivedCode; @@ -193,9 +194,8 @@ export class Http2CallStream extends Duplex implements CallStream { } delete headers['grpc-status']; } - let details = ''; - if (headers.hasOwnProperty('grpc-message')) { - details = decodeURI(headers['grpc-message']); + if (typeof headers['grpc-message'] === 'string') { + details = decodeURI(headers['grpc-message'] as string); } let metadata: Metadata; try { @@ -301,7 +301,7 @@ export class Http2CallStream extends Duplex implements CallStream { } this.endCall({code: code, details: details, metadata: new Metadata()}); }); - stream.on('error', () => { + stream.on('error', (err: Error) => { this.endCall({ code: Status.INTERNAL, details: 'Internal HTTP2 error', @@ -325,7 +325,9 @@ export class Http2CallStream extends Duplex implements CallStream { cancelWithStatus(status: Status, details: string): void { this.endCall({code: status, details: details, metadata: new Metadata()}); - if (this.http2Stream !== null) { + // The http2 stream could already have been destroyed if cancelWithStatus + // is called in response to an internal http2 error. + if (this.http2Stream !== null && !this.http2Stream.destroyed) { /* TODO(murgatroid99): Determine if we want to send different RST_STREAM * codes based on the status code */ this.http2Stream.rstWithCancel(); diff --git a/packages/grpc-js-core/src/metadata.ts b/packages/grpc-js-core/src/metadata.ts index 9ed69dfcc..c2ad6332a 100644 --- a/packages/grpc-js-core/src/metadata.ts +++ b/packages/grpc-js-core/src/metadata.ts @@ -166,6 +166,7 @@ export class Metadata { * Creates an OutgoingHttpHeaders object that can be used with the http2 API. */ toHttp2Headers(): http2.OutgoingHttpHeaders { + // NOTE: Node <8.9 formats http2 headers incorrectly. const result: http2.OutgoingHttpHeaders = {}; forOwn(this.internalRepr, (values, key) => { // We assume that the user's interaction with this object is limited to