Skip to content

V2: Ensure that a signal exists for a completed RPC #1282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
timostamm merged 4 commits into from
Oct 18, 2024
Merged

V2: Ensure that a signal exists for a completed RPC #1282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
adca40b
Revert "V2: Don't trigger handler signal on success (#1234)"
timostamm Oct 17, 2024
ad0f134
Keep propagating errors.
timostamm Oct 18, 2024
4d2fb86
Update JSDoc for clarity.
timostamm Oct 18, 2024
576d7e8
Keep propagating errors.
timostamm Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions packages/connect/src/implementation.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,12 @@ describe("createHandlerContext()", function () {
expect(ctx.signal.aborted).toBeTrue();
expect(ctx.signal.reason).toBe("shutdown-signal");
});
it("should trigger on abort with reason", function () {
it("should trigger on abort", 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 () {
Expand Down
9 changes: 4 additions & 5 deletions packages/connect/src/implementation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,9 @@ export interface HandlerContext {
readonly service: DescService;

/**
* An AbortSignal that triggers when the deadline is reached, or when an
* error occurs that aborts processing of the request.
* An AbortSignal that triggers when the deadline is reached, or when an error
* occurs that aborts processing of the request, but also when the RPC is
* completed without error.
*
* The signal can be used to automatically cancel downstream calls.
*/
Expand Down Expand Up @@ -175,9 +176,7 @@ export function createHandlerContext(
responseTrailer: new Headers(init.responseTrailer),
abort(reason?: unknown) {
deadline.cleanup();
if (reason !== undefined) {
abortController.abort(reason);
}
abortController.abort(reason);
},
values: init.contextValues ?? createContextValues(),
};
Expand Down