Refactor error handling in grpcwebclientbase

Author: stanley-cheung

javascript/net/grpc/web/BUILD.bazel

@@ -121,6 +121,7 @@ closure_js_library(
     visibility = ["//visibility:public"],
     deps = [
         ":clientreadablestream",
+        ":error",
         ":generictransportinterface",
         ":grpcwebstreamparser",
         ":status",

javascript/net/grpc/web/grpcwebclientbase.js

@@ -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);
@@ -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);
   }
 
   /**

javascript/net/grpc/web/grpcwebclientreadablestream.js

@@ -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');
@@ -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_ = [];
 
@@ -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);
@@ -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,
-              }));
+              });
             }
           }
         }
@@ -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;
       }
@@ -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) {
@@ -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
@@ -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++) {

net/grpc/gateway/examples/echo/commonjs-example/client.js

@@ -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+'"');
-      }
-    }
   }
 );
 

net/grpc/gateway/examples/echo/echoapp.js

@@ -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
@@ -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) {
@@ -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);
@@ -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);

net/grpc/gateway/examples/echo/echotest.html

@@ -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();