Skip to content

Per-method default timeout on transports #1805

Description

@u9g

Is your feature request related to a problem? Please describe.
I set per-RPC deadlines from a custom method option in my protos (deadline_ms), so a slow RPC like an upload gets minutes and everything else gets 10 seconds. defaultTimeoutMs (#799) only takes one number for the whole transport, and an interceptor can't help because the deadline signal is already made from timeoutMs before the interceptors run.

Today I wrap the Transport to fill in the timeout myself:

function withDeadlines(transport: Transport): Transport {
  return {
    unary: (method, signal, timeoutMs, header, input, contextValues) =>
      transport.unary(method, signal, timeoutMs ?? deadline(method), header, input, contextValues),
    stream: (method, signal, timeoutMs, header, input, contextValues) =>
      transport.stream(method, signal, timeoutMs ?? deadline(method), header, input, contextValues),
  };
}

It works, but it restates the Transport signatures and sits outside the options where the timeout default lives.

Describe the solution you'd like
Let defaultTimeoutMs also take a function of the method, for both Connect for Web and Connect for Node.js:

createConnectTransport({
  baseUrl,
  defaultTimeoutMs: (method: DescMethod) =>
    hasOption(method, deadline_ms) ? getOption(method, deadline_ms) : 10_000,
});

A per-call timeoutMs still wins, and <= 0 still means no timeout. The change would be the type in CommonTransportOptions and the web/node transport options, plus resolving the value in the places that read opt.defaultTimeoutMs today (the three protocol transports in @connectrpc/connect/protocol-* and the two in @connectrpc/connect-web). Widening the option type is backward compatible for anyone passing a number.

Describe alternatives you've considered

  • Let interceptors set the call's timeout. That needs the deadline signal to be created after the interceptors run, which is a much bigger change to runUnaryCall/runStreamingCall.
  • Keep wrapping the Transport, as above.

If this shape sounds fine, I'm happy to send the PR with tests and docs.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions