Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 41 additions & 0 deletions src/main/java/com/resend/core/net/RequestOptions.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.resend.core.net;

import java.time.Duration;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -10,6 +11,7 @@
public class RequestOptions {
private final String idempotencyKey;
private final Map<String, String> additionalHeaders;
private final Duration timeout;

/**
* Constructs a RequestOptions object using the provided builder.
Expand All @@ -19,6 +21,16 @@ public class RequestOptions {
public RequestOptions(Builder builder) {
this.idempotencyKey = builder.idempotencyKey;
this.additionalHeaders = Collections.unmodifiableMap(new HashMap<>(builder.additionalHeaders));
this.timeout = builder.timeout;
}

/**
* Get the timeout applied to this request.
*
* @return The timeout, or {@code null} to use the client's configured timeouts.
*/
public Duration getTimeout() {
return timeout;
}

/**
Expand Down Expand Up @@ -52,8 +64,11 @@ public static Builder builder() {
* Builder class for constructing RequestOptions objects.
*/
public static class Builder {
private static final Duration MAX_TIMEOUT = Duration.ofNanos(Long.MAX_VALUE);

private String idempotencyKey;
private final Map<String, String> additionalHeaders;
private Duration timeout;

/**
* Constructs a new Builder with empty additional headers map.
Expand Down Expand Up @@ -96,6 +111,32 @@ public Builder addAll(Map<String, String> headers) {
return this;
}

/**
* 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>
Comment on lines +118 to +120

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>

*
* <p>Only the built-in {@code HttpClient} honors this option; a custom {@code IHttpClient} may ignore it.</p>
*
* @param timeout The call timeout; {@link Duration#ZERO} disables only the call timeout.
* @return The builder instance.
* @throws IllegalArgumentException If the timeout is negative, or too large to be expressed in nanoseconds
* (more than {@code Long.MAX_VALUE} nanoseconds, about 292 years).
*/
public Builder timeout(Duration timeout) {
if (timeout != null && timeout.isNegative()) {
throw new IllegalArgumentException("timeout must not be negative, got: " + timeout);
}
Comment thread
kewynakshlley marked this conversation as resolved.
if (timeout != null && timeout.compareTo(MAX_TIMEOUT) > 0) {
throw new IllegalArgumentException("timeout must not exceed " + MAX_TIMEOUT + ", got: " + timeout);
}
this.timeout = timeout;
return this;
}

/**
* Build a new RequestOptions object.
*
Expand Down
99 changes: 33 additions & 66 deletions src/main/java/com/resend/core/net/impl/HttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.io.File;
import java.io.IOException;
import java.util.Map;
import java.util.concurrent.TimeUnit;

/**
* The built-in {@link IHttpClient}, backed by OkHttp.
Expand Down Expand Up @@ -112,20 +113,7 @@ public String getBaseUrl() {
@Override
public AbstractHttpResponse<String> perform(final String path, final String apiKey, final HttpMethod method, final String payload, MediaType mediaType) {

RequestBody requestBody = null;
if(payload != null) {
requestBody = RequestBody.create(payload, mediaType);
}

Request request = new Request.Builder()
.url(baseUrl + path)
.addHeader("Accept", "application/json")
.addHeader("User-Agent", USER_AGENT)
.addHeader("Authorization", "Bearer " + apiKey)
.method(method.name(), requestBody)
.build();

return execute(request);
return execute(buildRequest(path, apiKey, method, toRequestBody(payload, mediaType), null, null), null);
}

/**
Expand All @@ -148,28 +136,7 @@ public AbstractHttpResponse<String> perform(
final MediaType mediaType,
final Map<String,String> additionalHeaders) {

RequestBody requestBody = null;
if(payload != null) {
requestBody = RequestBody.create(payload, mediaType);
}

Request.Builder requestBuilder = new Request.Builder()
.url(baseUrl + path)
.addHeader("Accept", "application/json")
.addHeader("User-Agent", USER_AGENT)
.addHeader("Authorization", "Bearer " + apiKey)
.method(method.name(), requestBody);


if (additionalHeaders != null) {
for (Map.Entry<String,String> h : additionalHeaders.entrySet()) {
requestBuilder.addHeader(h.getKey(), h.getValue());
}
}

Request request = requestBuilder.build();

return execute(request);
return execute(buildRequest(path, apiKey, method, toRequestBody(payload, mediaType), additionalHeaders, null), null);
}

/**
Expand All @@ -191,32 +158,7 @@ public AbstractHttpResponse<String> perform(
final MediaType mediaType,
final RequestOptions requestOptions) {

RequestBody requestBody = null;
if(payload != null) {
requestBody = RequestBody.create(payload, mediaType);
}

Request.Builder requestBuilder = new Request.Builder()
.url(baseUrl + path)
.addHeader("Accept", "application/json")
.addHeader("User-Agent", USER_AGENT)
.addHeader("Authorization", "Bearer " + apiKey)
.method(method.name(), requestBody);

if (requestOptions != null) {
if (requestOptions.getIdempotencyKey() != null && !requestOptions.getIdempotencyKey().isEmpty()) {
requestBuilder.addHeader("Idempotency-Key", requestOptions.getIdempotencyKey());
}
if (requestOptions.getAdditionalHeaders() != null && !requestOptions.getAdditionalHeaders().isEmpty()) {
for (Map.Entry<String, String> entry : requestOptions.getAdditionalHeaders().entrySet()) {
requestBuilder.addHeader(entry.getKey(), entry.getValue());
}
}
}

Request request = requestBuilder.build();

return execute(request);
return execute(buildRequest(path, apiKey, method, toRequestBody(payload, mediaType), null, requestOptions), requestOptions);
}

/**
Expand Down Expand Up @@ -332,12 +274,33 @@ private AbstractHttpResponse<String> executeMultipart(
}
}

return execute(buildRequest(path, apiKey, method, bodyBuilder.build(), null, requestOptions), requestOptions);
}

private static RequestBody toRequestBody(final String payload, final MediaType mediaType) {
return payload == null ? null : RequestBody.create(payload, mediaType);
}

private Request buildRequest(
final String path,
final String apiKey,
final HttpMethod method,
final RequestBody body,
final Map<String, String> additionalHeaders,
final RequestOptions requestOptions) {

Request.Builder requestBuilder = new Request.Builder()
.url(baseUrl + path)
.addHeader("Accept", "application/json")
.addHeader("User-Agent", USER_AGENT)
.addHeader("Authorization", "Bearer " + apiKey)
.method(method.name(), bodyBuilder.build());
.method(method.name(), body);

if (additionalHeaders != null) {
for (Map.Entry<String, String> entry : additionalHeaders.entrySet()) {
requestBuilder.addHeader(entry.getKey(), entry.getValue());
}
}

if (requestOptions != null) {
if (requestOptions.getIdempotencyKey() != null && !requestOptions.getIdempotencyKey().isEmpty()) {
Expand All @@ -350,11 +313,15 @@ private AbstractHttpResponse<String> executeMultipart(
}
}

return execute(requestBuilder.build());
return requestBuilder.build();
}

private AbstractHttpResponse<String> execute(final Request request) {
try (Response response = httpClient.newCall(request).execute()) {
private AbstractHttpResponse<String> execute(final Request request, final RequestOptions requestOptions) {
Call call = httpClient.newCall(request);
if (requestOptions != null && requestOptions.getTimeout() != null) {
call.timeout().timeout(requestOptions.getTimeout().toNanos(), TimeUnit.NANOSECONDS);
}
try (Response response = call.execute()) {
return new AbstractHttpResponse<>(response.code(), response.body().string(), response.isSuccessful());
} catch (IOException e) {
throw new RuntimeException(e);
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/com/resend/core/service/BaseService.java
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ protected <T> T execute(final String path, final HttpMethod method, final String
* @param method The HTTP method.
* @param payload The body payload (or null).
* @param mediaType The media type for the payload.
* @param requestOptions The options with additional headers.
* @param requestOptions The per-request options, or {@code null} to send the request without any.
* @param responseType The class to deserialize the response body into.
* @param <T> The response type.
* @return The deserialized response.
Expand All @@ -107,6 +107,9 @@ protected <T> T execute(final String path, final HttpMethod method, final String
protected <T> T execute(final String path, final HttpMethod method, final String payload,
final MediaType mediaType, final RequestOptions requestOptions,
final Class<T> responseType) throws ResendException {
if (requestOptions == null) {
return execute(path, method, payload, mediaType, responseType);
}
return handle(httpClient.perform(path, apiKey, method, payload, mediaType, requestOptions), responseType);
}

Expand Down
136 changes: 135 additions & 1 deletion src/test/java/com/resend/core/net/impl/HttpClientTest.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
package com.resend.core.net.impl;

import okhttp3.OkHttpClient;
import com.resend.core.net.HttpMethod;
import com.resend.core.net.RequestOptions;
import okhttp3.*;
import org.junit.jupiter.api.Test;

import java.time.Duration;
import java.util.Collections;

import static org.junit.jupiter.api.Assertions.*;

public class HttpClientTest {
Expand Down Expand Up @@ -51,4 +56,133 @@ public void testBaseUrl_RejectsQueryAndFragment() {
public void testConstructor_RejectsNullOkHttpClient() {
assertThrows(IllegalArgumentException.class, () -> new HttpClient(null));
}

@Test
public void testPerform_RequestTimeoutOverridesClientCallTimeout() {
CapturingInterceptor capture = new CapturingInterceptor();
HttpClient client = clientWith(capture, Duration.ofSeconds(30));
RequestOptions options = RequestOptions.builder().timeout(Duration.ofMillis(1500)).build();

client.perform("/emails", "re_test", HttpMethod.GET, null, null, options);

assertEquals(Duration.ofMillis(1500).toNanos(), capture.callTimeoutNanos);
}

@Test
public void testPerform_WithoutRequestTimeoutUsesClientCallTimeout() {
CapturingInterceptor capture = new CapturingInterceptor();
HttpClient client = clientWith(capture, Duration.ofSeconds(30));

client.perform("/emails", "re_test", HttpMethod.GET, null, null, RequestOptions.builder().build());

assertEquals(Duration.ofSeconds(30).toNanos(), capture.callTimeoutNanos);
}

@Test
public void testPerform_ZeroRequestTimeoutDisablesCallTimeout() {
CapturingInterceptor capture = new CapturingInterceptor();
HttpClient client = clientWith(capture, Duration.ofSeconds(30));
RequestOptions options = RequestOptions.builder().timeout(Duration.ZERO).build();

client.perform("/emails", "re_test", HttpMethod.GET, null, null, options);

assertEquals(0L, capture.callTimeoutNanos);
}

@Test
public void testPerform_RequestTimeoutDoesNotLeakIntoLaterRequests() {
CapturingInterceptor capture = new CapturingInterceptor();
HttpClient client = clientWith(capture, Duration.ofSeconds(30));

client.perform("/emails", "re_test", HttpMethod.GET, null, null,
RequestOptions.builder().timeout(Duration.ofMillis(1500)).build());
client.perform("/emails", "re_test", HttpMethod.GET, null, null);

assertEquals(Duration.ofSeconds(30).toNanos(), capture.callTimeoutNanos);
}

@Test
public void testPerform_RequestOptionsStillAddHeaders() {
CapturingInterceptor capture = new CapturingInterceptor();
HttpClient client = clientWith(capture, Duration.ZERO);
RequestOptions options = RequestOptions.builder()
.setIdempotencyKey("key-1")
.add("X-Trace-Id", "trace-1")
.timeout(Duration.ofSeconds(2))
.build();

client.perform("/emails", "re_test", HttpMethod.POST, "{}", MediaType.get("application/json"), options);

assertEquals("key-1", capture.request.header("Idempotency-Key"));
assertEquals("trace-1", capture.request.header("X-Trace-Id"));
assertEquals("Bearer re_test", capture.request.header("Authorization"));
}

@Test
public void testPerformMultipart_RequestTimeoutIsApplied() {
CapturingInterceptor capture = new CapturingInterceptor();
HttpClient client = clientWith(capture, Duration.ofSeconds(30));
RequestOptions options = RequestOptions.builder().timeout(Duration.ofSeconds(5)).build();

client.performMultipart("/contacts/imports", "re_test", HttpMethod.POST, new byte[]{1, 2}, "contacts.csv",
MediaType.get("text/csv"), Collections.<String, String>emptyMap(), options);

assertEquals(Duration.ofSeconds(5).toNanos(), capture.callTimeoutNanos);
}

@Test
public void testRequestOptions_RejectsNegativeTimeout() {
assertThrows(IllegalArgumentException.class,
() -> RequestOptions.builder().timeout(Duration.ofSeconds(-1)));
}

@Test
public void testRequestOptions_RejectsTimeoutThatOverflowsNanoseconds() {
assertThrows(IllegalArgumentException.class,
() -> RequestOptions.builder().timeout(Duration.ofDays(200_000)));
assertThrows(IllegalArgumentException.class,
() -> RequestOptions.builder().timeout(Duration.ofNanos(Long.MAX_VALUE).plusNanos(1)));
}

@Test
public void testRequestOptions_AcceptsLargestRepresentableTimeout() {
Duration largest = Duration.ofNanos(Long.MAX_VALUE);

RequestOptions options = RequestOptions.builder().timeout(largest).build();

assertEquals(largest, options.getTimeout());
assertEquals(Long.MAX_VALUE, options.getTimeout().toNanos());
}

@Test
public void testRequestOptions_TimeoutDefaultsToNull() {
assertNull(RequestOptions.builder().build().getTimeout());
}

private static HttpClient clientWith(final CapturingInterceptor capture, final Duration callTimeout) {
OkHttpClient okHttp = new OkHttpClient.Builder()
.callTimeout(callTimeout)
.addInterceptor(capture)
.build();
return new HttpClient(okHttp, "http://localhost:8080");
}

private static final class CapturingInterceptor implements Interceptor {

private Request request;
private long callTimeoutNanos = -1L;

@Override
public Response intercept(final Chain chain) {
request = chain.request();
callTimeoutNanos = chain.call().timeout().timeoutNanos();
return new Response.Builder()
.request(chain.request())
.protocol(Protocol.HTTP_1_1)
.code(200)
.message("stub")
.body(ResponseBody.create("{}", MediaType.get("application/json")))
.build();
}
}
}
Loading