> handler) {
- // Use stream to handle large responses safely
+ // Buffer entire response in memory; check size limits in handleResponse to mitigate large responses.
if (requestBody != null) {
- Buffer buffer = requestEncoding != null ? Buffer.buffer(requestBody, requestEncoding) : Buffer.buffer(requestBody);
+ Buffer buffer;
+ try {
+ buffer = requestEncoding != null ? Buffer.buffer(requestBody, requestEncoding) : Buffer.buffer(requestBody);
+ } catch (IllegalArgumentException e) {
+ handler.handle(io.vertx.core.Future.failedFuture(new HttpRequestException("Invalid encoding: " + requestEncoding, e)));
+ return;
+ }
request.sendBuffer(buffer, ar -> handleResponse(ar, handler));
} else {
request.send(ar -> handleResponse(ar, handler));
@@ -271,11 +285,11 @@ public void send(final ICompleteListener completeListener) {
}
} else {
log.error(ar.cause().getLocalizedMessage(), ar.cause());
- // Attempt to notify listener of failure via a 500 response if possible,
+ // Attempt to notify listener of failure via a 503 response.
// strictly speaking ICompleteListener expects a response.
ResponseWrapper errorResponse = new ResponseWrapper();
- errorResponse.setHttpCode(500);
- errorResponse.setHttpCodeMessage(ar.cause().getLocalizedMessage());
+ errorResponse.setHttpCode(503);
+ errorResponse.setHttpCodeMessage("Service Unavailable: " + ar.cause().getLocalizedMessage());
try {
completeListener.onComplete(errorResponse);
} catch (IResponse.HttpResponseException e) {
From 253edb08489616da1e9b5509eef4582474ead064 Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
<161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Sat, 22 Nov 2025 15:09:01 +0000
Subject: [PATCH 6/6] Address final review feedback for Vert.x WebClient
migration
- HttpClientModule:
- Store and close the underlying WebClient to ensure proper resource management.
- Improve idle timeout conversion logic (use ceil for milliseconds).
- HttpClientWrapper:
- Unwrap ExecutionException to expose root cause.
- URL-decode query parameters during parsing.
- Remove unused requestContentType field.
- Clarify and condense comments regarding buffering and Basic Auth limitations.
- Add warning about non-reusability of RequestWrapper.
- VertxHttpClient:
- Added field to hold underlying WebClient.
---
.../bootstrap/HttpClientModule.java | 15 +++++----
.../httpclient/impl/HttpClientWrapper.java | 32 ++++++++++++-------
.../httpclient/impl/VertxHttpClient.java | 2 ++
3 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/src/main/java/ai/labs/eddi/engine/httpclient/bootstrap/HttpClientModule.java b/src/main/java/ai/labs/eddi/engine/httpclient/bootstrap/HttpClientModule.java
index 757f81cdc..79af88e19 100644
--- a/src/main/java/ai/labs/eddi/engine/httpclient/bootstrap/HttpClientModule.java
+++ b/src/main/java/ai/labs/eddi/engine/httpclient/bootstrap/HttpClientModule.java
@@ -30,9 +30,12 @@ public VertxHttpClient provideHttpClient(@ConfigProperty(name = "httpClient.maxC
options.setMaxPoolSize(maxConnectionPerRoute);
options.setMaxRedirects(maxRedirects);
- int idleTimeoutSeconds = idleTimeout / 1000;
- if (idleTimeout > 0 && idleTimeoutSeconds == 0) {
- idleTimeoutSeconds = 1;
+
+ int idleTimeoutSeconds;
+ if (idleTimeout == 0) {
+ idleTimeoutSeconds = 0;
+ } else {
+ idleTimeoutSeconds = (int) Math.ceil(idleTimeout / 1000.0);
}
options.setIdleTimeout(idleTimeoutSeconds);
@@ -43,12 +46,12 @@ public VertxHttpClient provideHttpClient(@ConfigProperty(name = "httpClient.maxC
WebClient webClient = WebClient.create(vertx, options);
WebClientSession webClientSession = WebClientSession.create(webClient);
- return new VertxHttpClient(vertx, webClientSession);
+ return new VertxHttpClient(vertx, webClientSession, webClient);
}
public void close(@Disposes VertxHttpClient client) {
- if (client.getWebClient() != null) {
- client.getWebClient().close();
+ if (client.getUnderlyingClient() != null) {
+ client.getUnderlyingClient().close();
}
}
}
diff --git a/src/main/java/ai/labs/eddi/engine/httpclient/impl/HttpClientWrapper.java b/src/main/java/ai/labs/eddi/engine/httpclient/impl/HttpClientWrapper.java
index bb51b0a28..1b4167554 100644
--- a/src/main/java/ai/labs/eddi/engine/httpclient/impl/HttpClientWrapper.java
+++ b/src/main/java/ai/labs/eddi/engine/httpclient/impl/HttpClientWrapper.java
@@ -18,7 +18,9 @@
import org.jboss.logging.Logger;
import java.net.URI;
+import java.net.URLDecoder;
import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
import java.util.*;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
@@ -63,6 +65,14 @@ public IRequest newRequest(URI uri, Method method) {
return new RequestWrapper(uri, request, vertxMethod);
}
+ /**
+ * Wrapper for Vert.x HttpRequest.
+ *
+ * Note: This class is stateful and wraps a mutable {@link HttpRequest}.
+ * It is designed to be used for a single request configuration and execution.
+ * Reusing an instance of this class for multiple {@code send()} calls may result in
+ * accumulated headers or query parameters.
+ */
@Getter
@EqualsAndHashCode
private class RequestWrapper implements IRequest {
@@ -72,7 +82,6 @@ private class RequestWrapper implements IRequest {
private int maxLength = 8 * 1024 * 1024;
private String requestBody;
private String requestEncoding;
- private String requestContentType;
private long currentTimeout = 60000; // Default timeout fallback
private final Map> queryParamsMap = new HashMap<>();
@@ -89,6 +98,10 @@ private class RequestWrapper implements IRequest {
int idx = pair.indexOf(KEY_EQUALS);
String key = idx > 0 ? pair.substring(0, idx) : pair;
String value = idx > 0 && pair.length() > idx + 1 ? pair.substring(idx + 1) : null;
+
+ if (key != null) key = URLDecoder.decode(key, StandardCharsets.UTF_8);
+ if (value != null) value = URLDecoder.decode(value, StandardCharsets.UTF_8);
+
queryParamsMap.computeIfAbsent(key, k -> new ArrayList<>()).add(value);
}
}
@@ -99,6 +112,7 @@ public IRequest setBasicAuthentication(String username, String password, String
// Vert.x basic auth helper
// Note: 'realm' and 'preemptive' parameters are ignored by Vert.x WebClient's basicAuthentication helper.
// It automatically sets the Authorization header (equivalent to preemptive=true).
+ // WARNING: Non-preemptive authentication (preemptive=false) is NOT supported in this implementation.
request.basicAuthentication(username, password);
return this;
}
@@ -125,7 +139,6 @@ public IRequest setUserAgent(String userAgent) {
public IRequest setBodyEntity(String content, String encoding, String contentType) {
this.requestBody = content;
this.requestEncoding = encoding;
- this.requestContentType = contentType;
if (contentType != null) {
request.putHeader("Content-Type", contentType);
@@ -136,14 +149,8 @@ public IRequest setBodyEntity(String content, String encoding, String contentTyp
@Override
public IRequest setMaxResponseSize(int maxLength) {
this.maxLength = maxLength;
- // Vert.x doesn't have a direct "max response size" on the request builder that limits buffering
- // automatically in the same way as Jetty's response listener might, but we can inspect content-length
- // or handle it in a stream if needed. However, for standard buffer response, it just buffers.
- // If we needed strict limiting, we might need to use sendStream and count bytes.
- // For now, we will assume the buffer handles it or we check afterwards.
- // Actually, `as(BodyCodec.string())` or similar doesn't limit.
- // Given the previous implementation used `BufferingResponseListener` with a limit,
- // we should be careful. But Vert.x WebClient buffers everything by default.
+ // Note: Vert.x WebClient buffers the entire response by default.
+ // Size limits are validated in handleResponse() after the response is received.
return this;
}
@@ -177,7 +184,8 @@ public IResponse send() throws HttpRequestException {
if (e instanceof InterruptedException) {
Thread.currentThread().interrupt();
}
- throw new HttpRequestException(e.getLocalizedMessage(), e);
+ Throwable cause = (e instanceof ExecutionException && e.getCause() != null) ? e.getCause() : e;
+ throw new HttpRequestException(cause.getLocalizedMessage(), cause);
}
}
@@ -286,7 +294,7 @@ public void send(final ICompleteListener completeListener) {
} else {
log.error(ar.cause().getLocalizedMessage(), ar.cause());
// Attempt to notify listener of failure via a 503 response.
- // strictly speaking ICompleteListener expects a response.
+ // Strictly speaking ICompleteListener expects a response.
ResponseWrapper errorResponse = new ResponseWrapper();
errorResponse.setHttpCode(503);
errorResponse.setHttpCodeMessage("Service Unavailable: " + ar.cause().getLocalizedMessage());
diff --git a/src/main/java/ai/labs/eddi/engine/httpclient/impl/VertxHttpClient.java b/src/main/java/ai/labs/eddi/engine/httpclient/impl/VertxHttpClient.java
index 47a5d03c1..b3954db93 100644
--- a/src/main/java/ai/labs/eddi/engine/httpclient/impl/VertxHttpClient.java
+++ b/src/main/java/ai/labs/eddi/engine/httpclient/impl/VertxHttpClient.java
@@ -1,6 +1,7 @@
package ai.labs.eddi.engine.httpclient.impl;
import io.vertx.core.Vertx;
+import io.vertx.ext.web.client.WebClient;
import io.vertx.ext.web.client.WebClientSession;
import lombok.AllArgsConstructor;
import lombok.Getter;
@@ -14,4 +15,5 @@
public class VertxHttpClient {
private Vertx vertx;
private WebClientSession webClient;
+ private WebClient underlyingClient;
}