Skip to content
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

Support timeouts in clients #635

Merged
merged 2 commits into from
May 16, 2023
Merged

Conversation

timostamm
Copy link
Member

@timostamm timostamm commented May 16, 2023

In #591, we added support for timeouts in handlers. For better reliability, timeouts are now honored in clients as well.

For example, consider the following client call:

elizaClient.say({ sentence: "Hello" }, { timeoutMs: 2500 });

The timeout of 2.5 seconds is sent to the server in a request header (this works for the gRPC, gRPC-web, and for the Connect protocol). If the timeout is reached before the server could produce a response in time, it will send an error with Code.DeadlineExceeded instead.

But if this error does not reach the client (for example because a connection became unresponsive), the client would wait for a response well past the timeout. With this PR, the timeout is not just sent as a header, but it is also honored in the client. If no response arrives before the timeout, the client will raise an error with Code.DeadlineExceeded itself, and stop the request.

The logic is implemented in the functions runUnaryCall and runStreamingCall from @bufbuild/connect/protocol, so that Transport implementations do not have to care about this detail themselves. Transports implementations also no longer have to catch errors - runUnaryCall() and runStreamingCall() catch errors and convert them to a Connect error if necessary.

The old versions of the functions - runUnary and runStreaming from @bufbuild/connect - are now marked deprecated.

Comment on lines -74 to -75
const ac = createLinkedAbortController(signal);
return await runUnary<I, O>(
Copy link
Member Author

Choose a reason for hiding this comment

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

The new run* functions handle abort signals and timeouts. They also trigger the abort signal if the Transport raises an error. This saves quite a bit of plumbing in Transport implementations, and seems worth the change.

@timostamm timostamm requested a review from smaye81 May 16, 2023 11:07
@timostamm timostamm merged commit 9ba933e into main May 16, 2023
@timostamm timostamm deleted the tstamm/handle-deadlines-on-the-client branch May 16, 2023 14:31
@timostamm timostamm mentioned this pull request Sep 6, 2023
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