Fix callback client tests

Signed-off-by: Sri Krishna Paritala <[email protected]>
connectrpc · Sep 4, 2024 · 04287b5 · 04287b5
1 parent 81f48c7
commit 04287b5
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 11 deletions.
diff --git a/packages/connect-conformance/src/callback-client.ts b/packages/connect-conformance/src/callback-client.ts
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import { createCallbackClient, ConnectError } from "@connectrpc/connect";
+import { createCallbackClient, ConnectError, Code } from "@connectrpc/connect";
 import type { CallbackClient, Transport } from "@connectrpc/connect";
 import {
   ClientCompatRequest,
@@ -85,6 +85,19 @@ async function unary(
   await wait(req.requestDelayMs);
   return new Promise<ClientResponseResult>((resolve) => {
     const controller = new AbortController();
+    // Handles client cancellation.
+    controller.signal.addEventListener("abort", () => {
+      resolve(
+        new ClientResponseResult({
+          payloads: payloads,
+          responseHeaders: resHeaders,
+          responseTrailers: resTrailers,
+          error: convertToProtoError(
+            error ?? new ConnectError("operation aborted", Code.Canceled),
+          ),
+        }),
+      );
+    });
     const { afterCloseSendMs } = getCancelTiming(req);
     if (afterCloseSendMs >= 0) {
       wait(afterCloseSendMs)
@@ -94,6 +107,11 @@ async function unary(
     call(
       uReq,
       (err, uRes) => {
+        // Callback clients swallow client triggered cancellations and never
+        // call the callback. This will trigger the global error handler and fail the process.
+        if (controller.signal.aborted) {
+          throw new Error("Aborted requests should not trigger the callback");
+        }
         if (err !== undefined) {
           error = ConnectError.from(err);
           // We can't distinguish between headers and trailers here, so we just
@@ -156,16 +174,22 @@ async function serverStream(
 
   await wait(req.requestDelayMs);
   return new Promise<ClientResponseResult>((resolve) => {
-    const cancelFn = client.serverStream(
+    const controller = new AbortController();
+    client.serverStream(
       uReq,
       (uResp) => {
         payloads.push(uResp.payload!);
         count++;
         if (count === cancelTiming.afterNumResponses) {
-          cancelFn();
+          controller.abort();
         }
       },
       (err) => {
+        // Callback clients swallow client triggered cancellations don't report
+        // that as an error.
+        if (err === undefined && controller.signal.aborted) {
+          error = new ConnectError("operation aborted", Code.Canceled);
+        }
         if (err !== undefined) {
           error = ConnectError.from(err);
           // We can't distinguish between headers and trailers here, so we just
@@ -189,6 +213,7 @@ async function serverStream(
       },
       {
         headers: reqHeader,
+        signal: controller.signal,
         onHeader(headers) {
           resHeaders = convertToProtoHeaders(headers);
         },
@@ -199,7 +224,7 @@ async function serverStream(
     );
     if (cancelTiming.afterCloseSendMs >= 0) {
       wait(cancelTiming.afterCloseSendMs)
-        .then(() => cancelFn())
+        .then(() => controller.abort())
         .catch(() => {});
     }
   });

diff --git a/packages/connect-web/conformance/known-failing-callback-client.txt b/packages/connect-web/conformance/known-failing-callback-client.txt
diff --git a/packages/connect-web/package.json b/packages/connect-web/package.json
@@ -14,13 +14,13 @@
     "build:esm": "tsc --project tsconfig.build.json --outDir ./dist/esm --declaration --declarationDir ./dist/esm",
     "conformance:safari": "npm run conformance:safari:promise && npm run conformance:client:safari:callback",
     "conformance:safari:promise": "connectconformance --mode client --conf conformance/conformance-web.yaml -- tsx conformance/client.ts --browser safari",
-    "conformance:safari:callback": "connectconformance --mode client --conf conformance/conformance-web.yaml --known-failing @conformance/known-failing-callback-client.txt -- tsx conformance/client.ts --browser safari --useCallbackClient",
+    "conformance:safari:callback": "connectconformance --mode client --conf conformance/conformance-web.yaml -- tsx conformance/client.ts --browser safari --useCallbackClient",
     "conformance:chrome:promise": "connectconformance --mode client --conf conformance/conformance-web.yaml -- tsx conformance/client.ts --browser chrome",
-    "conformance:chrome:callback": "connectconformance --mode client --conf conformance/conformance-web.yaml --known-failing @conformance/known-failing-callback-client.txt -- tsx conformance/client.ts --browser chrome --useCallbackClient",
+    "conformance:chrome:callback": "connectconformance --mode client --conf conformance/conformance-web.yaml -- tsx conformance/client.ts --browser chrome --useCallbackClient",
     "conformance:firefox:promise": "connectconformance --mode client --conf conformance/conformance-web.yaml -- tsx conformance/client.ts --browser firefox",
-    "conformance:firefox:callback": "connectconformance --mode client --conf conformance/conformance-web.yaml --known-failing @conformance/known-failing-callback-client.txt -- tsx conformance/client.ts --browser firefox --useCallbackClient",
+    "conformance:firefox:callback": "connectconformance --mode client --conf conformance/conformance-web.yaml -- tsx conformance/client.ts --browser firefox --useCallbackClient",
     "conformance:node:promise": "connectconformance --mode client --conf conformance/conformance-web-node.yaml -- tsx conformance/client.ts --browser node",
-    "conformance:node:callback": "connectconformance --mode client --conf conformance/conformance-web-node.yaml --known-failing @conformance/known-failing-callback-client.txt -- tsx conformance/client.ts --browser node --useCallbackClient",
+    "conformance:node:callback": "connectconformance --mode client --conf conformance/conformance-web-node.yaml -- tsx conformance/client.ts --browser node --useCallbackClient",
     "test": "jasmine --config=jasmine.json",
     "generate": "buf generate --template browserstack/buf.gen.yaml",
     "postgenerate": "license-header browserstack/gen",