Skip to content

Commit

Permalink
Refactor error handling in grpcwebclientbase
Browse files Browse the repository at this point in the history
  • Loading branch information
stanley-cheung committed Jun 22, 2020
1 parent daf7cc8 commit a8416b5
Show file tree
Hide file tree
Showing 7 changed files with 248 additions and 80 deletions.
1 change: 1 addition & 0 deletions javascript/net/grpc/web/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ closure_js_library(
visibility = ["//visibility:public"],
deps = [
":clientreadablestream",
":error",
":generictransportinterface",
":grpcwebstreamparser",
":status",
Expand Down
41 changes: 40 additions & 1 deletion javascript/net/grpc/web/grpcwebclientbase.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,45 @@ class GrpcWebClientBase {
* @export
*/
rpcCall(method, requestMessage, metadata, methodDescriptor, callback) {
/**
* @implements {ClientReadableStream}
*/
class ClientUnaryCallImpl {
/**
* @param {!ClientReadableStream<RESPONSE>} stream
* @template RESPONSE
*/
constructor(stream) {
this.stream = stream;
}

/**
* @override
*/
on(eventType, callback) {
if (eventType == 'data' || eventType == 'error') {
// unary call responses and errors should be handled by the main
// (err, resp) => ... callback
return this;
}
return this.stream.on(eventType, callback);
}

/**
* @override
*/
removeListener(eventType, callback) {
return this.stream.removeListener(eventType, callback);
}

/**
* @override
*/
cancel() {
this.stream.cancel();
}
}

methodDescriptor = AbstractClientBase.ensureMethodDescriptor(
method, requestMessage, MethodType.UNARY, methodDescriptor);
var hostname = AbstractClientBase.getHostname(method, methodDescriptor);
Expand All @@ -105,7 +144,7 @@ class GrpcWebClientBase {
var stream = /** @type {!ClientReadableStream<?>} */ (invoker.call(
this, methodDescriptor.createRequest(requestMessage, metadata)));
GrpcWebClientBase.setCallback_(stream, callback, false);
return stream;
return new ClientUnaryCallImpl(stream);
}

/**
Expand Down
47 changes: 32 additions & 15 deletions javascript/net/grpc/web/grpcwebclientreadablestream.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ goog.module.declareLegacyNamespace();
const ClientReadableStream = goog.require('grpc.web.ClientReadableStream');
const ErrorCode = goog.require('goog.net.ErrorCode');
const EventType = goog.require('goog.net.EventType');
const GrpcWebError = goog.require('grpc.web.Error');
const GrpcWebStreamParser = goog.require('grpc.web.GrpcWebStreamParser');
const StatusCode = goog.require('grpc.web.StatusCode');
const XhrIo = goog.require('goog.net.XhrIo');
Expand Down Expand Up @@ -103,7 +104,7 @@ class GrpcWebClientReadableStream {
/**
* @const
* @private
* @type {!Array<function(...):?>} The list of error callbacks
* @type {!Array<function(!GrpcWebError)>} The list of error callbacks
*/
this.onErrorCallbacks_ = [];

Expand Down Expand Up @@ -150,6 +151,11 @@ class GrpcWebClientReadableStream {
var byteSource = new Uint8Array(
/** @type {!ArrayBuffer} */ (self.xhr_.getResponse()));
} else {
self.handleError_({
code: StatusCode.UNKNOWN,
message: 'Unknown Content-type received.',
metadata: {},
});
return;
}
var messages = self.parser_.parse(byteSource);
Expand Down Expand Up @@ -184,11 +190,11 @@ class GrpcWebClientReadableStream {
grpcStatusMessage = trailers[GRPC_STATUS_MESSAGE];
delete trailers[GRPC_STATUS_MESSAGE];
}
self.sendStatusCallbacks_(/** @type {!Status} */ ({
self.handleError_({
code: Number(grpcStatusCode),
details: grpcStatusMessage,
message: decodeURIComponent(grpcStatusMessage),
metadata: trailers,
}));
});
}
}
}
Expand Down Expand Up @@ -227,9 +233,10 @@ class GrpcWebClientReadableStream {
if (grpcStatusCode == StatusCode.ABORTED && self.aborted_) {
return;
}
self.sendErrorCallbacks_({
self.handleError_({
code: grpcStatusCode,
message: ErrorCode.getDebugMessage(lastErrorCode)
message: ErrorCode.getDebugMessage(lastErrorCode),
metadata: {},
});
return;
}
Expand All @@ -243,20 +250,13 @@ class GrpcWebClientReadableStream {
grpcStatusMessage = self.xhr_.getResponseHeader(GRPC_STATUS_MESSAGE);
}
if (Number(grpcStatusCode) != StatusCode.OK) {
self.sendErrorCallbacks_({
self.handleError_({
code: Number(grpcStatusCode),
message: grpcStatusMessage,
metadata: responseHeaders
});
errorEmitted = true;
}
if (!errorEmitted) {
self.sendStatusCallbacks_(/** @type {!Status} */ ({
code: Number(grpcStatusCode),
details: grpcStatusMessage,
metadata: responseHeaders
}));
}
}

if (!errorEmitted) {
Expand Down Expand Up @@ -353,6 +353,23 @@ class GrpcWebClientReadableStream {
return headers;
}

/**
* A central place to handle errors
*
* @private
* @param {!GrpcWebError} error The error object
*/
handleError_(error) {
if (error.code != StatusCode.OK) {
this.sendErrorCallbacks_(error);
}
this.sendStatusCallbacks_(/** @type {!Status} */ ({
code: error.code,
details: decodeURIComponent(error.message || ''),
metadata: error.metadata
}));
}

/**
* @private
* @param {!RESPONSE} data The data to send back
Expand Down Expand Up @@ -385,7 +402,7 @@ class GrpcWebClientReadableStream {

/**
* @private
* @param {?} error The error to send back
* @param {!GrpcWebError} error The error to send back
*/
sendErrorCallbacks_(error) {
for (var i = 0; i < this.onErrorCallbacks_.length; i++) {
Expand Down
8 changes: 0 additions & 8 deletions net/grpc/gateway/examples/echo/commonjs-example/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,6 @@ var echoApp = new EchoApp(
{
EchoRequest: EchoRequest,
ServerStreamingEchoRequest: ServerStreamingEchoRequest
},
{
checkGrpcStatusCode: function(status) {
if (status.code != grpc.web.StatusCode.OK) {
EchoApp.addRightMessage('Error code: '+status.code+' "'+
status.details+'"');
}
}
}
);

Expand Down
8 changes: 1 addition & 7 deletions net/grpc/gateway/examples/echo/echoapp.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@ const echoapp = {};
/**
* @param {Object} echoService
* @param {Object} ctors
* @param {Object} handlers
*/
echoapp.EchoApp = function(echoService, ctors, handlers) {
echoapp.EchoApp = function(echoService, ctors) {
this.echoService = echoService;
this.ctors = ctors;
this.handlers = handlers;
};

echoapp.EchoApp.INTERVAL = 500; // ms
Expand Down Expand Up @@ -64,7 +62,6 @@ echoapp.EchoApp.prototype.echo = function(msg) {
echoapp.EchoApp.addLeftMessage(msg);
var unaryRequest = new this.ctors.EchoRequest();
unaryRequest.setMessage(msg);
var self = this;
var call = this.echoService.echo(unaryRequest,
{"custom-header-1": "value1"},
function(err, response) {
Expand All @@ -78,7 +75,6 @@ echoapp.EchoApp.prototype.echo = function(msg) {
}
});
call.on('status', function(status) {
self.handlers.checkGrpcStatusCode(status);
if (status.metadata) {
console.log("Received metadata");
console.log(status.metadata);
Expand Down Expand Up @@ -118,12 +114,10 @@ echoapp.EchoApp.prototype.repeatEcho = function(msg, count) {
var stream = this.echoService.serverStreamingEcho(
streamRequest,
{"custom-header-1": "value1"});
var self = this;
stream.on('data', function(response) {
echoapp.EchoApp.addRightMessage(response.getMessage());
});
stream.on('status', function(status) {
self.handlers.checkGrpcStatusCode(status);
if (status.metadata) {
console.log("Received metadata");
console.log(status.metadata);
Expand Down
7 changes: 0 additions & 7 deletions net/grpc/gateway/examples/echo/echotest.html
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,6 @@
{
EchoRequest: proto.grpc.gateway.testing.EchoRequest,
ServerStreamingEchoRequest: proto.grpc.gateway.testing.ServerStreamingEchoRequest
},
{
checkGrpcStatusCode: function(status) {
if (status.code != grpc.web.StatusCode.OK) {
echoapp.EchoApp.addRightMessage('Error code: '+status.code+' "'+status.details+'"');
}
}
}
);
echoApp.load();
Expand Down
Loading

0 comments on commit a8416b5

Please sign in to comment.