V2: Don't trigger handler signal on success (#1234)

Signed-off-by: Sri Krishna Paritala <[email protected]>
connectrpc · Sep 17, 2024 · 25ca96e · 25ca96e
1 parent ed4b0e5
commit 25ca96e
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 7 deletions.
diff --git a/packages/connect/src/implementation.spec.ts b/packages/connect/src/implementation.spec.ts
@@ -68,12 +68,17 @@ describe("createHandlerContext()", function () {
       expect(ctx.signal.aborted).toBeTrue();
       expect(ctx.signal.reason).toBe("shutdown-signal");
     });
-    it("should trigger on abort", function () {
+    it("should trigger on abort with reason", function () {
       const ctx = createHandlerContext({ ...standardOptions });
       ctx.abort("test-reason");
       expect(ctx.signal.aborted).toBeTrue();
       expect(ctx.signal.reason).toBe("test-reason");
     });
+    it("should not trigger on abort without reason", function () {
+      const ctx = createHandlerContext({ ...standardOptions });
+      ctx.abort();
+      expect(ctx.signal.aborted).toBeFalse();
+    });
   });
 
   describe("timeout()", function () {

diff --git a/packages/connect/src/implementation.ts b/packages/connect/src/implementation.ts
@@ -70,8 +70,8 @@ export interface HandlerContext {
   readonly service: DescService;
 
   /**
-   * An AbortSignal that is aborted when the connection with the client is closed
-   * or when the deadline is reached.
+   * An AbortSignal that triggers when the deadline is reached, or when an
+   * error occurs that aborts processing of the request.
    *
    * The signal can be used to automatically cancel downstream calls.
    */
@@ -177,7 +177,9 @@ export function createHandlerContext(
     responseTrailer: new Headers(init.responseTrailer),
     abort(reason?: unknown) {
       deadline.cleanup();
-      abortController.abort(reason);
+      if (reason !== undefined) {
+        abortController.abort(reason);
+      }
     },
     values: init.contextValues ?? createContextValues(),
   };

diff --git a/packages/connect/src/protocol-connect/handler-factory.ts b/packages/connect/src/protocol-connect/handler-factory.ts
@@ -258,6 +258,7 @@ function createUnaryHandler<I extends DescMessage, O extends DescMessage>(
       );
       body = serialization.getO(type.binary).serialize(output);
     } catch (e) {
+      context.abort(e);
       let error: ConnectError | undefined;
       if (e instanceof ConnectError) {
         error = e;
@@ -462,7 +463,7 @@ function createStreamHandler<I extends DescMessage, O extends DescMessage>(
       },
       transformSerializeEnvelope(serialization.getO(type.binary)),
       transformCatchFinally<EnvelopedMessage>((e) => {
-        context.abort();
+        context.abort(e);
         const end: EndStreamResponse = {
           metadata: context.responseTrailer,
         };

diff --git a/packages/connect/src/protocol-grpc-web/handler-factory.ts b/packages/connect/src/protocol-grpc-web/handler-factory.ts
@@ -199,7 +199,7 @@ function createHandler<I extends DescMessage, O extends DescMessage>(
       },
       transformSerializeEnvelope(serialization.getO(type.binary)),
       transformCatchFinally<EnvelopedMessage>((e) => {
-        context.abort();
+        context.abort(e);
         if (e instanceof ConnectError) {
           setTrailerStatus(context.responseTrailer, e);
         } else if (e !== undefined) {

diff --git a/packages/connect/src/protocol-grpc/handler-factory.ts b/packages/connect/src/protocol-grpc/handler-factory.ts
@@ -187,7 +187,7 @@ function createHandler<I extends DescMessage, O extends DescMessage>(
       transformCompressEnvelope(compression.response, opt.compressMinBytes),
       transformJoinEnvelopes(),
       transformCatchFinally<Uint8Array>((e): void => {
-        context.abort();
+        context.abort(e);
         if (e instanceof ConnectError) {
           setTrailerStatus(context.responseTrailer, e);
         } else if (e !== undefined) {