Skip to content

Commit

Permalink
Feature: option to retrieve original json body if parse exception occ…
Browse files Browse the repository at this point in the history
…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]>
  • Loading branch information
3 people authored Oct 17, 2024
1 parent 2a4d7dd commit 659ed36
Show file tree
Hide file tree
Showing 10 changed files with 262 additions and 12 deletions.
20 changes: 20 additions & 0 deletions docs/release-notes/release-highlights.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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() {
}
Expand All @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -377,6 +381,7 @@ private <ResponseT> ResponseT decodeTransportResponse(
) throws IOException {

if (endpoint instanceof JsonEndpoint) {

@SuppressWarnings("unchecked")
JsonEndpoint<?, ResponseT, ?> jsonEndpoint = (JsonEndpoint<?, ResponseT, ?>) endpoint;
// Successful response
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ public class RestClientOptions implements TransportOptions {

private final RequestOptions options;

boolean keepResponseBodyOnException;

@VisibleForTesting
static final String CLIENT_META_VALUE = getClientMeta();
@VisibleForTesting
Expand All @@ -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();
}

Expand Down Expand Up @@ -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());
Expand All @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ public Function<List<String>, Boolean> onWarnings() {
return null;
}

@Override
public boolean keepResponseBodyOnException() {
return false;
}

@Override
public Builder toBuilder() {
return null;
Expand Down
Loading

0 comments on commit 659ed36

Please sign in to comment.