From 04287b52268b40b94432abb575cc33c1c4e5c2f4 Mon Sep 17 00:00:00 2001 From: Sri Krishna Paritala Date: Wed, 4 Sep 2024 14:30:20 +0530 Subject: [PATCH] Fix callback client tests Signed-off-by: Sri Krishna Paritala --- .../src/callback-client.ts | 33 ++++++++++++++++--- .../known-failing-callback-client.txt | 3 -- packages/connect-web/package.json | 8 ++--- 3 files changed, 33 insertions(+), 11 deletions(-) delete mode 100644 packages/connect-web/conformance/known-failing-callback-client.txt diff --git a/packages/connect-conformance/src/callback-client.ts b/packages/connect-conformance/src/callback-client.ts index d778b3e99..c21e2070e 100644 --- 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((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((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 deleted file mode 100644 index ea1718118..000000000 --- a/packages/connect-web/conformance/known-failing-callback-client.txt +++ /dev/null @@ -1,3 +0,0 @@ -# The callback client does not pass a cancelled error to the end-of-stream callback for cancellations. This is intentional behavior as it is the user's -# responsibility to handle this on the client side. -**/server-stream/cancel-after-responses diff --git a/packages/connect-web/package.json b/packages/connect-web/package.json index 8d49a71be..c92e5f07b 100644 --- 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",