-
Notifications
You must be signed in to change notification settings - Fork 276
Feature: option to retrieve original json body if parse exception occurred #886
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
51976d1
35c07e9
9835bc4
0e93afd
d003cc6
5bcf666
9832695
890a76b
cdeb192
e7eee8a
30ee626
435d2ef
0beeebb
d93cd29
d416490
a7b422e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
---|---|---|
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should keep the existing constructor without the additional parameter to avoid a breaking change (and default the additional parameter to Also, to be future proof if we add more options in the features, what about changing this constructor to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the future proof constructor will make more sense when we'll have control over RequestOptions in the low level client! |
||
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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is it empty? It should set a boolean flag in the builder, and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it cannot set a boolean flag in the builder, because the builder is in the RestClient code |
||
} | ||
|
||
@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) { | ||
|
Uh oh!
There was an error while loading. Please reload this page.