Skip to content

feat(core): add per-request timeout to RequestOptions - #154

Open
kewynakshlley wants to merge 5 commits into
mainfrom
feat/request-timeout
Open

kewynakshlley wants to merge 5 commits into
mainfrom
feat/request-timeout

Conversation

@kewynakshlley

@kewynakshlley kewynakshlley commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • RequestOptions: new timeout(Duration). It rejects negatives, ZERO means no timeout, and null falls back to the client's callTimeout.
  • HttpClient: the request-building code that was copied across the perform overloads and executeMultipart is now one buildRequest helper. execute applies the per-request timeout on the call itself, so it covers each attempt and doesn't create a new client or leak into later requests.
  • BaseService.execute(..., RequestOptions, ...): a null options argument now falls back to the 5-arg perform. That keeps the existing mock-based service tests working and is what lets PR 3 make every no-options method delegate to its options overload.
  • Tests in HttpClientTest: a per-request timeout overrides the client's, a missing one falls back to it, ZERO disables it, it doesn't carry over to the next request, multipart honors it, headers and the idempotency key still go out, and negative values are rejected.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread src/main/java/com/resend/core/net/RequestOptions.java

@felipefreitag felipefreitag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

call.timeout() sets only OkHttp’s call timeout. The client’s connect, read and write timeouts still apply, and their default is 10 s. So timeout(Duration.ofSeconds(60)) still fails when the server is silent for 10 s, and Duration.ZERO removes only the call timeout. I think we should update the javadoc on RequestOptions.Builder.timeout to say this.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Confidence score: 4/5

  • RequestOptions.java describes the connect, read, and write timeouts as limits on total request duration, which can mislead callers because a request may run longer while making progress. Clarify what each timeout limits.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/main/java/com/resend/core/net/RequestOptions.java">

<violation number="1" location="src/main/java/com/resend/core/net/RequestOptions.java:118">
P2: This incorrectly describes the connect, read, and write timeouts as caps on total request duration; a request can exceed them while connection and data transfers continue to make progress. Clarify that they limit connection setup and individual transfer waits, not the overall call duration.</violation>
</file>

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment on lines +118 to +120
* <p>The client's connect, read and write timeouts (10 seconds each by default) still apply, so this option
* can shorten a request but can't make it wait longer than those limits. To allow longer requests, raise
* them with {@link com.resend.Resend.Builder}.</p>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: This incorrectly describes the connect, read, and write timeouts as caps on total request duration; a request can exceed them while connection and data transfers continue to make progress. Clarify that they limit connection setup and individual transfer waits, not the overall call duration.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/main/java/com/resend/core/net/RequestOptions.java, line 118:

<comment>This incorrectly describes the connect, read, and write timeouts as caps on total request duration; a request can exceed them while connection and data transfers continue to make progress. Clarify that they limit connection setup and individual transfer waits, not the overall call duration.</comment>

<file context>
@@ -112,12 +112,16 @@ public Builder addAll(Map<String, String> headers) {
+         * Set a call timeout for this request, bounding the whole call from connecting to reading the full response.
          * It overrides the client's {@code callTimeout} for this request only.
          *
+         * <p>The client's connect, read and write timeouts (10 seconds each by default) still apply, so this option
+         * can shorten a request but can't make it wait longer than those limits. To allow longer requests, raise
+         * them with {@link com.resend.Resend.Builder}.</p>
</file context>
Suggested change
* <p>The client's connect, read and write timeouts (10 seconds each by default) still apply, so this option
* can shorten a request but can't make it wait longer than those limits. To allow longer requests, raise
* them with {@link com.resend.Resend.Builder}.</p>
* <p>The client's connect, read and write timeouts (10 seconds each by default) still apply independently: they limit connection setup and individual waits for data or request-body writes, not total request duration. Raise
* them with {@link com.resend.Resend.Builder} if those waits need to be longer.</p>

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