From e56bdbf34b9ac95f49c3b299182c502d9f43b721 Mon Sep 17 00:00:00 2001 From: Dave Gramlich Date: Thu, 21 Jul 2016 19:38:27 -0400 Subject: [PATCH 1/4] grpc: listen for metadata event instead of status --- lib/common/grpc-service.js | 3 ++- test/common/grpc-service.js | 27 +++++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/lib/common/grpc-service.js b/lib/common/grpc-service.js index 4f50f6eb5c8..32d42bff083 100644 --- a/lib/common/grpc-service.js +++ b/lib/common/grpc-service.js @@ -325,7 +325,8 @@ GrpcService.prototype.requestStream = function(protoOpts, reqOpts) { request: function() { return service[protoOpts.method](reqOpts, grpcOpts) - .on('status', function(status) { + .on('metadata', function() { + var status = this.received_status || { code: 0 }; var grcpStatus = GrpcService.decorateStatus_(status); this.emit('response', grcpStatus || status); diff --git a/test/common/grpc-service.js b/test/common/grpc-service.js index d04b039bc4c..6d998c5c684 100644 --- a/test/common/grpc-service.js +++ b/test/common/grpc-service.js @@ -1062,7 +1062,7 @@ describe('GrpcService', function() { ); }); - it('should emit the status as a response event', function(done) { + it('should emit the metadata event as a response event', function(done) { var grpcError500 = { code: 2 }; var fakeStream = through.obj(); @@ -1074,6 +1074,8 @@ describe('GrpcService', function() { return options.request(); }; + fakeStream.received_status = grpcError500; + fakeStream .on('response', function(resp) { assert.deepEqual(resp, GrpcService.GRPC_ERROR_CODE_TO_HTTP[2]); @@ -1081,7 +1083,28 @@ describe('GrpcService', function() { }); grpcService.requestStream(PROTO_OPTS, REQ_OPTS); - fakeStream.emit('status', grpcError500); + fakeStream.emit('metadata'); + }); + + it('should default to OK status when metadata fires', function(done) { + var fakeStream = through.obj(); + + ProtoService.prototype.method = function() { + return fakeStream; + }; + + retryRequestOverride = function(reqOpts, options) { + return options.request(); + }; + + fakeStream + .on('response', function(resp) { + assert.deepEqual(resp, GrpcService.GRPC_ERROR_CODE_TO_HTTP[0]); + done(); + }); + + grpcService.requestStream(PROTO_OPTS, REQ_OPTS); + fakeStream.emit('metadata'); }); it('should emit the response error', function(done) { From 4032b4315dbac9bd2e7b3c672bc28ceb2e54d846 Mon Sep 17 00:00:00 2001 From: Dave Gramlich Date: Thu, 21 Jul 2016 19:44:51 -0400 Subject: [PATCH 2/4] listening for error events in unit tests --- test/common/grpc-service.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/common/grpc-service.js b/test/common/grpc-service.js index 6d998c5c684..17c30843f40 100644 --- a/test/common/grpc-service.js +++ b/test/common/grpc-service.js @@ -1077,6 +1077,7 @@ describe('GrpcService', function() { fakeStream.received_status = grpcError500; fakeStream + .on('error', done) .on('response', function(resp) { assert.deepEqual(resp, GrpcService.GRPC_ERROR_CODE_TO_HTTP[2]); done(); @@ -1098,6 +1099,7 @@ describe('GrpcService', function() { }; fakeStream + .on('error', done) .on('response', function(resp) { assert.deepEqual(resp, GrpcService.GRPC_ERROR_CODE_TO_HTTP[0]); done(); From d70602eb81332b6cee988472f6ce25a33ad0d388 Mon Sep 17 00:00:00 2001 From: Dave Gramlich Date: Fri, 22 Jul 2016 14:04:57 -0400 Subject: [PATCH 3/4] setting default status to OK --- lib/common/grpc-service.js | 5 ++--- test/common/grpc-service.js | 25 ------------------------- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/lib/common/grpc-service.js b/lib/common/grpc-service.js index 32d42bff083..21dcfa3a6a0 100644 --- a/lib/common/grpc-service.js +++ b/lib/common/grpc-service.js @@ -326,10 +326,9 @@ GrpcService.prototype.requestStream = function(protoOpts, reqOpts) { request: function() { return service[protoOpts.method](reqOpts, grpcOpts) .on('metadata', function() { - var status = this.received_status || { code: 0 }; - var grcpStatus = GrpcService.decorateStatus_(status); + var grcpStatus = GrpcService.decorateStatus_({ code: 0 }); - this.emit('response', grcpStatus || status); + this.emit('response', grcpStatus); }); } }; diff --git a/test/common/grpc-service.js b/test/common/grpc-service.js index 17c30843f40..d9be9c26a69 100644 --- a/test/common/grpc-service.js +++ b/test/common/grpc-service.js @@ -1063,31 +1063,6 @@ describe('GrpcService', function() { }); it('should emit the metadata event as a response event', function(done) { - var grpcError500 = { code: 2 }; - var fakeStream = through.obj(); - - ProtoService.prototype.method = function() { - return fakeStream; - }; - - retryRequestOverride = function(reqOpts, options) { - return options.request(); - }; - - fakeStream.received_status = grpcError500; - - fakeStream - .on('error', done) - .on('response', function(resp) { - assert.deepEqual(resp, GrpcService.GRPC_ERROR_CODE_TO_HTTP[2]); - done(); - }); - - grpcService.requestStream(PROTO_OPTS, REQ_OPTS); - fakeStream.emit('metadata'); - }); - - it('should default to OK status when metadata fires', function(done) { var fakeStream = through.obj(); ProtoService.prototype.method = function() { From 43a1c0d08198da176ff517fdbe07c67bc9bec3ac Mon Sep 17 00:00:00 2001 From: Dave Gramlich Date: Fri, 22 Jul 2016 14:10:53 -0400 Subject: [PATCH 4/4] adding comment about stubbed status code --- lib/common/grpc-service.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/common/grpc-service.js b/lib/common/grpc-service.js index 21dcfa3a6a0..f537e6dc781 100644 --- a/lib/common/grpc-service.js +++ b/lib/common/grpc-service.js @@ -326,6 +326,12 @@ GrpcService.prototype.requestStream = function(protoOpts, reqOpts) { request: function() { return service[protoOpts.method](reqOpts, grpcOpts) .on('metadata', function() { + // retry-request requires a server response before it starts emitting + // data. The closest mechanism grpc provides is a metadata event, but + // this does not provide any kind of response status. So we're faking + // it here with code `0` which translates to HTTP 200. + // + // https://github.com/GoogleCloudPlatform/gcloud-node/pull/1444#discussion_r71812636 var grcpStatus = GrpcService.decorateStatus_({ code: 0 }); this.emit('response', grcpStatus);