Skip to content

feat: throw DeadlineExceeded caused by AbortSignal.timeout #1454

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

polRk
Copy link

@polRk polRk commented Mar 23, 2025

No description provided.

@polRk polRk changed the title feat: add support for DeadlineExceeded AbortSignal.timeout feat: add support for DeadlineExceeded by AbortSignal.timeout Mar 23, 2025
@polRk polRk changed the title feat: add support for DeadlineExceeded by AbortSignal.timeout feat: throw DeadlineExceeded caused by AbortSignal.timeout Mar 23, 2025
Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! We'll have to use Code.Canceled, see comments below.

Can you add test below https://github.com/connectrpc/connect-es/blob/v2.0.2/packages/connect/src/connect-error.spec.ts#L157?

    it("wraps AbortError with code canceled", () => {
      // abort() on AbortSignal aborts with a AbortError
      const error: unknown = new DOMException("foo", "AbortError");
      const got = ConnectError.from(error);
      expect(got.code).toBe(Code.Canceled);
      expect(got.rawMessage).toBe("foo");
    });
    it("wraps TimeoutError with code canceled", () => {
      // AbortSignal.timeout() aborts with a TimeoutError
      const error: unknown = new DOMException("foo", "TimeoutError");
      const got = ConnectError.from(error);
      expect(got.code).toBe(Code.Canceled);
      expect(got.rawMessage).toBe("foo");
    });

Please run npx turbo run format bundle-size to update formatting and bundle-size benchmarks.

Comment on lines +124 to +127
if (reason.name == "TimeoutError") {
// AbortSignal.timeout(0)
return new ConnectError(reason.message, Code.DeadlineExceeded);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to use Code.Canceled for "TimeoutError", see #1453 (comment).

Can you merge this if-statement with the one above? We can simply check for reason.name == "AbortError" || reason.name == "TimeoutError".

Please add to the comments:

        // Calling abort() on AbortSignal aborts with a AbortError.
        // AbortSignal.timeout() aborts with a TimeoutError.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants