Feature: option to retrieve original json body if parse exception occ…

…urred (#886) (#898) * base working impl * twr, remove unused constructor * unit test * license * complete refactor * reverting old changes * codestyle * leave ElasticsearchTransportBase untouched * Update java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java * Update java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java * custom generic response, checking in transport base * license header * asciidocs --------- Co-authored-by: Laura Trotta <[email protected]> Co-authored-by: Sylvain Wallez <[email protected]>
elastic · Oct 17, 2024 · 659ed36 · 659ed36
1 parent 2a4d7dd
commit 659ed36
Show file tree

Hide file tree

Showing 10 changed files with 262 additions and 12 deletions.
diff --git a/docs/release-notes/release-highlights.asciidoc b/docs/release-notes/release-highlights.asciidoc
@@ -8,6 +8,26 @@ For a list of detailed changes, including bug fixes, please see the https://gith
 [discrete]
 ==== Version 8.16
 * `ElasticsearchClient` is now `Closeable`. Closing a client object also closes the underlying transport - https://github.com/elastic/elasticsearch-java/pull/851[#851]
+* Added option to make the response body available in case of deserialization error- https://github.com/elastic/elasticsearch-java/pull/886[#886].
+
+** While it has always been possible to set the log level to `trace` and have the client print both the json bodies of the requests and responses, it's often not the best solution because of the large amount of information printed.
+** To enable the feature:
+
+    RestClientOptions options = new RestClientOptions(RequestOptions.DEFAULT, true);
+    ElasticsearchTransport transport = new RestClientTransport(restClient, new JacksonJsonpMapper(), options);
+    ElasticsearchClient esClientWithOptions = new ElasticsearchClient(transport);
+
+** To retrieve the original body from the TransportException that gets thrown in case of deserialization errors:
+
+    try{
+        // some code that returns faulty json
+    }
+    catch (TransportException ex){
+        try (RepeatableBodyResponse repeatableResponse = (RepeatableBodyResponse) ex.response()) {
+            BinaryData body = repeatableResponse.body();
+        }
+    }
+
 
 [discrete]
 ==== Version 8.15

diff --git a/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java b/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java
@@ -37,13 +37,24 @@ public class DefaultTransportOptions implements TransportOptions {
     private final HeaderMap headers;
     private final Map<String, String> parameters;
     private final Function<List<String>, Boolean> onWarnings;
+    private boolean keepResponseBodyOnException;
 
     public static final DefaultTransportOptions EMPTY = new DefaultTransportOptions();
 
     public DefaultTransportOptions() {
         this(new HeaderMap(), Collections.emptyMap(), null);
     }
 
+    public DefaultTransportOptions(
+        @Nullable HeaderMap headers,
+        @Nullable Map<String, String> parameters,
+        @Nullable Function<List<String>, Boolean> onWarnings,
+        boolean keepResponseBodyOnException
+    ) {
+        this(headers,parameters,onWarnings);
+        this.keepResponseBodyOnException = keepResponseBodyOnException;
+    }
+
     public DefaultTransportOptions(
         @Nullable HeaderMap headers,
         @Nullable Map<String, String> parameters,
@@ -53,10 +64,11 @@ public DefaultTransportOptions(
         this.parameters = (parameters == null || parameters.isEmpty()) ?
             Collections.emptyMap() : Collections.unmodifiableMap(parameters);
         this.onWarnings = onWarnings;
+        this.keepResponseBodyOnException = false;
     }
 
     protected DefaultTransportOptions(AbstractBuilder<?> builder) {
-        this(builder.headers, builder.parameters, builder.onWarnings);
+        this(builder.headers, builder.parameters, builder.onWarnings, builder.keepResponseBodyOnException);
     }
 
     public static DefaultTransportOptions of(@Nullable TransportOptions options) {
@@ -88,6 +100,11 @@ public Function<List<String>, Boolean> onWarnings() {
         return onWarnings;
     }
 
+    @Override
+    public boolean keepResponseBodyOnException() {
+        return keepResponseBodyOnException;
+    }
+
     @Override
     public Builder toBuilder() {
         return new Builder(this);
@@ -111,6 +128,7 @@ public abstract static class AbstractBuilder<BuilderT extends AbstractBuilder<Bu
         private HeaderMap headers;
         private Map<String, String> parameters;
         private Function<List<String>, Boolean> onWarnings;
+        private boolean keepResponseBodyOnException;
 
         public AbstractBuilder() {
         }
@@ -119,10 +137,17 @@ public AbstractBuilder(DefaultTransportOptions options) {
             this.headers = new HeaderMap(options.headers);
             this.parameters = copyOrNull(options.parameters);
             this.onWarnings = options.onWarnings;
+            this.keepResponseBodyOnException = options.keepResponseBodyOnException;
         }
 
         protected abstract BuilderT self();
 
+        @Override
+        public BuilderT keepResponseBodyOnException(boolean value) {
+            this.keepResponseBodyOnException = value;
+            return self();
+        }
+
         @Override
         public BuilderT addHeader(String name, String value) {
             if (name.equalsIgnoreCase(HeaderMap.CLIENT_META)) {

diff --git a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java
@@ -29,14 +29,15 @@
 import co.elastic.clients.transport.endpoints.BooleanEndpoint;
 import co.elastic.clients.transport.endpoints.BooleanResponse;
 import co.elastic.clients.transport.http.HeaderMap;
+import co.elastic.clients.transport.http.RepeatableBodyResponse;
 import co.elastic.clients.transport.http.TransportHttpClient;
 import co.elastic.clients.transport.instrumentation.Instrumentation;
 import co.elastic.clients.transport.instrumentation.NoopInstrumentation;
 import co.elastic.clients.transport.instrumentation.OpenTelemetryForElasticsearch;
+import co.elastic.clients.util.ByteArrayBinaryData;
 import co.elastic.clients.util.LanguageRuntimeVersions;
 import co.elastic.clients.util.ApiTypeHelper;
 import co.elastic.clients.util.BinaryData;
-import co.elastic.clients.util.ByteArrayBinaryData;
 import co.elastic.clients.util.ContentType;
 import co.elastic.clients.util.MissingRequiredPropertyException;
 import co.elastic.clients.util.NoCopyByteArrayOutputStream;
@@ -306,6 +307,9 @@ private <ResponseT, ErrorT> ResponseT getApiResponse(
 
         int statusCode = clientResp.statusCode();
 
+        if(options().keepResponseBodyOnException()){
+            clientResp = RepeatableBodyResponse.of(clientResp);
+        }
         try {
             if (statusCode == 200) {
                 checkProductHeader(clientResp, endpoint);
@@ -377,6 +381,7 @@ private <ResponseT> ResponseT decodeTransportResponse(
     ) throws IOException {
 
         if (endpoint instanceof JsonEndpoint) {
+
             @SuppressWarnings("unchecked")
             JsonEndpoint<?, ResponseT, ?> jsonEndpoint = (JsonEndpoint<?, ResponseT, ?>) endpoint;
             // Successful response

diff --git a/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java b/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java
@@ -38,6 +38,13 @@ public interface TransportOptions {
 
     Function<List<String>, Boolean> onWarnings();
 
+    /**
+     * If {@code true}, the response body in {@code TransportException.response().body()} is guaranteed to be
+     * replayable (i.e. buffered), even if the original response was streamed. This allows inspecting the
+     * response body in case of error.
+     */
+    boolean keepResponseBodyOnException();
+
     Builder toBuilder();
 
     default TransportOptions with(Consumer<Builder> fn) {
@@ -59,5 +66,12 @@ interface Builder extends ObjectBuilder<TransportOptions> {
         Builder removeParameter(String name);
 
         Builder onWarnings(Function<List<String>, Boolean> listener);
+
+        /**
+         * Should the response body be buffered and made available in {@code TransportException.response().body()}?
+         * This setting guarantees that the response body is buffered for inspection if parsing fails, even if originally
+         * streamed by the http library.
+         */
+        Builder keepResponseBodyOnException(boolean value);
     }
 }
diff --git a/java-client/src/main/java/co/elastic/clients/transport/http/RepeatableBodyResponse.java b/java-client/src/main/java/co/elastic/clients/transport/http/RepeatableBodyResponse.java
@@ -0,0 +1,84 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package co.elastic.clients.transport.http;
+
+import co.elastic.clients.util.BinaryData;
+import co.elastic.clients.util.ByteArrayBinaryData;
+
+import javax.annotation.Nullable;
+import java.io.IOException;
+import java.util.List;
+
+public class RepeatableBodyResponse implements TransportHttpClient.Response {
+
+    private final TransportHttpClient.Response response;
+    private final BinaryData body;
+
+    public static TransportHttpClient.Response of(TransportHttpClient.Response response) throws IOException {
+        BinaryData body = response.body();
+        if (body == null || body.isRepeatable()) {
+            return response;
+        }
+        return new RepeatableBodyResponse(response);
+    }
+
+    public RepeatableBodyResponse(TransportHttpClient.Response response) throws IOException {
+        this.response = response;
+        this.body = new ByteArrayBinaryData(response.body());
+    }
+
+    @Override
+    public TransportHttpClient.Node node() {
+        return response.node();
+    }
+
+    @Override
+    public int statusCode() {
+        return response.statusCode();
+    }
+
+    @Nullable
+    @Override
+    public String header(String name) {
+        return response.header(name);
+    }
+
+    @Override
+    public List<String> headers(String name) {
+        return response.headers(name);
+    }
+
+    @Nullable
+    @Override
+    public BinaryData body() throws IOException {
+        return this.body;
+    }
+
+    @Nullable
+    @Override
+    public Object originalResponse() {
+        return response.originalResponse();
+    }
+
+    @Override
+    public void close() throws IOException {
+        response.close();
+    }
+}
diff --git a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java
@@ -85,7 +85,8 @@ public RestClientOptions createOptions(@Nullable TransportOptions options) {
     }
 
     @Override
-    public Response performRequest(String endpointId, @Nullable Node node, Request request, TransportOptions options) throws IOException {
+    public Response performRequest(String endpointId, @Nullable Node node, Request request,
+                                   TransportOptions options) throws IOException {
         RestClientOptions rcOptions = RestClientOptions.of(options);
         org.elasticsearch.client.Request restRequest = createRestRequest(request, rcOptions);
         org.elasticsearch.client.Response restResponse = restClient.performRequest(restRequest);
@@ -103,7 +104,7 @@ public CompletableFuture<Response> performRequestAsync(
         try {
             RestClientOptions rcOptions = RestClientOptions.of(options);
             restRequest = createRestRequest(request, rcOptions);
-        } catch(Throwable thr) {
+        } catch (Throwable thr) {
             // Terminate early
             future.completeExceptionally(thr);
             return future;
@@ -166,7 +167,7 @@ private org.elasticsearch.client.Request createRestRequest(Request request, Rest
         if (body != null) {
             ContentType ct = null;
             String ctStr;
-            if (( ctStr = requestHeaders.get(HeaderMap.CONTENT_TYPE)) != null) {
+            if ((ctStr = requestHeaders.get(HeaderMap.CONTENT_TYPE)) != null) {
                 ct = ContentTypeCache.computeIfAbsent(ctStr, ContentType::parse);
             }
             clientReq.setEntity(new MultiBufferEntity(body, ct));

diff --git a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java
@@ -42,6 +42,8 @@ public class RestClientOptions implements TransportOptions {
 
     private final RequestOptions options;
 
+    boolean keepResponseBodyOnException;
+
     @VisibleForTesting
     static final String CLIENT_META_VALUE = getClientMeta();
     @VisibleForTesting
@@ -63,7 +65,8 @@ static RestClientOptions of(@Nullable TransportOptions options) {
         return builder.build();
     }
 
-    public RestClientOptions(RequestOptions options) {
+    public RestClientOptions(RequestOptions options, boolean keepResponseBodyOnException) {
+        this.keepResponseBodyOnException = keepResponseBodyOnException;
         this.options = addBuiltinHeaders(options.toBuilder()).build();
     }
 
@@ -99,6 +102,11 @@ public Function<List<String>, Boolean> onWarnings() {
         return warnings -> options.getWarningsHandler().warningsShouldFailRequest(warnings);
     }
 
+    @Override
+    public boolean keepResponseBodyOnException() {
+        return this.keepResponseBodyOnException;
+    }
+
     @Override
     public Builder toBuilder() {
         return new Builder(options.toBuilder());
@@ -108,6 +116,8 @@ public static class Builder implements TransportOptions.Builder {
 
         private RequestOptions.Builder builder;
 
+        private boolean keepResponseBodyOnException;
+
         public Builder(RequestOptions.Builder builder) {
             this.builder = builder;
         }
@@ -181,14 +191,20 @@ public TransportOptions.Builder onWarnings(Function<List<String>, Boolean> liste
             return this;
         }
 
+        @Override
+        public TransportOptions.Builder keepResponseBodyOnException(boolean value) {
+            this.keepResponseBodyOnException = value;
+            return this;
+        }
+
         @Override
         public RestClientOptions build() {
-            return new RestClientOptions(addBuiltinHeaders(builder).build());
+            return new RestClientOptions(addBuiltinHeaders(builder).build(), keepResponseBodyOnException);
         }
     }
 
     static RestClientOptions initialOptions() {
-        return new RestClientOptions(SafeResponseConsumer.DEFAULT_REQUEST_OPTIONS);
+        return new RestClientOptions(SafeResponseConsumer.DEFAULT_REQUEST_OPTIONS, false);
     }
 
     private static RequestOptions.Builder addBuiltinHeaders(RequestOptions.Builder builder) {

diff --git a/java-client/src/test/java/co/elastic/clients/documentation/DocTestsTransport.java b/java-client/src/test/java/co/elastic/clients/documentation/DocTestsTransport.java
@@ -64,6 +64,11 @@ public Function<List<String>, Boolean> onWarnings() {
             return null;
         }
 
+        @Override
+        public boolean keepResponseBodyOnException() {
+            return false;
+        }
+
         @Override
         public Builder toBuilder() {
             return null;