refactor: improve sendStatus a little when there is no metadata involved

grpc · Oct 19, 2022 · c24c213 · c24c213
1 parent d124214
commit c24c213
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 13 deletions.
diff --git a/packages/grpc-js/src/call-interface.ts b/packages/grpc-js/src/call-interface.ts
@@ -36,6 +36,10 @@ export interface StatusObject {
   metadata: Metadata;
 }
 
+export type PartialStatusObject = Pick<StatusObject, 'code' | 'details'> & {
+  metadata: Metadata | null;
+}
+
 export const enum WriteFlags {
   BufferHint = 1,
   NoCompress = 2,
@@ -166,4 +170,4 @@ export interface Call {
   halfClose(): void;
   getCallNumber(): number;
   setCredentials(credentials: CallCredentials): void;
-}
+}
diff --git a/packages/grpc-js/src/server-call.ts b/packages/grpc-js/src/server-call.ts
@@ -33,7 +33,7 @@ import { StreamDecoder } from './stream-decoder';
 import { ObjectReadable, ObjectWritable } from './object-stream';
 import { ChannelOptions } from './channel-options';
 import * as logging from './logging';
-import { StatusObject } from './call-interface';
+import { PartialStatusObject, StatusObject } from './call-interface';
 import { Deadline } from './deadline';
 
 const TRACER_NAME = 'server_call';
@@ -441,9 +441,12 @@ export class Http2ServerCallStream<
         this.sendStatus({
           code: Status.CANCELLED,
           details: 'Cancelled by client',
-          metadata: new Metadata(),
+          metadata: null,
         });
       }
+
+      // to compensate for a fact that cancelled is not always called
+      this.emit('close');
     });
 
     this.stream.on('drain', () => {
@@ -497,7 +500,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 = { ...defaultResponseHeaders, ...custom };
     this.stream.respond(headers, defaultResponseOptions);
   }
 
@@ -654,18 +657,19 @@ export class Http2ServerCallStream<
   async sendUnaryMessage(
     err: ServerErrorResponse | ServerStatusResponse | null,
     value?: ResponseType | null,
-    metadata?: Metadata,
+    metadata?: Metadata | null,
     flags?: number
   ) {
     if (this.checkCancelled()) {
       return;
     }
-    if (!metadata) {
-      metadata = new Metadata();
+
+    if (metadata === undefined) {
+      metadata = null;
     }
 
     if (err) {
-      if (!Object.prototype.hasOwnProperty.call(err, 'metadata')) {
+      if (!Object.prototype.hasOwnProperty.call(err, 'metadata') && metadata) {
         err.metadata = metadata;
       }
       this.sendError(err);
@@ -683,7 +687,7 @@ export class Http2ServerCallStream<
     }
   }
 
-  sendStatus(statusObj: StatusObject) {
+  sendStatus(statusObj: PartialStatusObject) {
     this.emit('callEnd', statusObj.code);
     this.emit('streamEnd', statusObj.code === Status.OK);
     if (this.checkCancelled()) {
@@ -707,7 +711,7 @@ export class Http2ServerCallStream<
         const trailersToSend = {
           [GRPC_STATUS_HEADER]: statusObj.code,
           [GRPC_MESSAGE_HEADER]: encodeURI(statusObj.details),
-          ...statusObj.metadata.toHttp2Headers(),
+          ...statusObj.metadata?.toHttp2Headers(),
         };
 
         this.stream.sendTrailers(trailersToSend);
@@ -719,13 +723,13 @@ export class Http2ServerCallStream<
   }
 
   sendError(error: ServerErrorResponse | ServerStatusResponse) {
-    const status: StatusObject = {
+    const status: PartialStatusObject = {
       code: Status.UNKNOWN,
       details: 'message' in error ? error.message : 'Unknown Error',
       metadata:
         'metadata' in error && error.metadata !== undefined
           ? error.metadata
-          : new Metadata(),
+          : null,
     };
 
     if (
@@ -774,6 +778,9 @@ export class Http2ServerCallStream<
       call.emit('cancelled', reason);
     });
 
+    // to compensate for the fact that cancelled is no longer always called
+    this.once('close', () => call.emit('close'))
+
     this.once('callEnd', (status) => call.emit('callEnd', status));
   }
 

diff --git a/packages/grpc-js/src/server.ts b/packages/grpc-js/src/server.ts
@@ -81,7 +81,6 @@ function getUnimplementedStatusResponse(
   return {
     code: Status.UNIMPLEMENTED,
     details: `The server does not implement the method ${methodName}`,
-    metadata: new Metadata(),
   };
 }