You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "arturobernalg (via GitHub)" <gi...@apache.org> on 2023/03/11 21:24:51 UTC
[GitHub] [httpcomponents-client] arturobernalg opened a new pull request, #423: Add FaultTolerantResponseHandler class and use it in execute and returnContent methods
arturobernalg opened a new pull request, #423:
URL: https://github.com/apache/httpcomponents-client/pull/423
This PR adds a new class called `FaultTolerantResponseHandler` that extends `ContentResponseHandler`. It provides a custom response handler that reads the entity from the response body and transforms it into a `Content` object. This handler is designed to be fault-tolerant, meaning that it can handle responses with null or empty bodies without throwing exceptions. If the response was successful (a 2xx status code), the `Content` object is returned. If no response body exists, `Content.NO_CONTENT` is returned.
This implementation extends `ContentResponseHandler` and overrides its methods to handle the response entity and transform it into the actual response object. This is designed to be efficient and use minimal memory.
The `FaultTolerantResponseHandler` is intended to be used in scenarios where the response body may be null or empty, but the caller still needs to determine whether the response was successful or not.
In addition, this PR also updates the `execute` and `returnContent` methods in the existing codebase to use the new `FaultTolerantResponseHandler`. Specifically, the `execute` method now takes an instance of `FaultTolerantResponseHandler` as an argument, and the `returnContent` method now calls the `handleResponse` method with an instance of `FaultTolerantResponseHandler`.
This change will make the code more fault-tolerant and efficient in handling responses with null or empty bodies.
This PR is backwards compatible with existing code, and all existing tests pass with these changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] arturobernalg commented on pull request #423: Add FaultTolerantResponseHandler class and use it in execute and returnContent methods
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #423:
URL: https://github.com/apache/httpcomponents-client/pull/423#issuecomment-1465166388
> @arturobernalg Please do it simpler and just enhance `ContentResponseHandler` instead of creating a whole new handler.
done. add it into `ContentResponseHandler`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on pull request #423: Add FaultTolerantResponseHandler class and use it in execute and returnContent methods
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #423:
URL: https://github.com/apache/httpcomponents-client/pull/423#issuecomment-1465145187
@arturobernalg Please do it simpler and just enhance `ContentResponseHandler` instead of creating a whole new handler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on a diff in pull request #423: Enhances ContentResponseHandler to be fault-tolerant
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #423:
URL: https://github.com/apache/httpcomponents-client/pull/423#discussion_r1138607416
##########
httpclient5-fluent/src/main/java/org/apache/hc/client5/http/fluent/ContentResponseHandler.java:
##########
@@ -26,29 +26,57 @@
*/
package org.apache.hc.client5.http.fluent;
-import java.io.IOException;
-
+import org.apache.hc.client5.http.HttpResponseException;
import org.apache.hc.client5.http.impl.classic.AbstractHttpClientResponseHandler;
+import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.io.entity.EntityUtils;
+import java.io.IOException;
Review Comment:
@arturobernalg Please do not let imports move around unnecessarily. It is just useless noise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on a diff in pull request #423: Enhances ContentResponseHandler to be fault-tolerant
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #423:
URL: https://github.com/apache/httpcomponents-client/pull/423#discussion_r1133252971
##########
httpclient5-fluent/src/main/java/org/apache/hc/client5/http/fluent/ContentResponseHandler.java:
##########
@@ -44,11 +46,63 @@
*/
public class ContentResponseHandler extends AbstractHttpClientResponseHandler<Content> {
+ /**
+ * Handles the response entity and transforms it into a {@link Content} object. If the response entity is null or
+ * has zero length, {@link Content#NO_CONTENT} is returned.
+ *
+ * @param entity the response entity.
+ * @return a {@link Content} object that encapsulates the response body, or {@link Content#NO_CONTENT} if the
+ * response body is null or has zero length.
+ * @throws IOException if an I/O error occurs.
+ */
@Override
public Content handleEntity(final HttpEntity entity) throws IOException {
- return entity != null ?
- new Content(EntityUtils.toByteArray(entity), ContentType.parse(entity.getContentType())) :
- Content.NO_CONTENT;
+ if (entity != null && entity.getContentLength() > 0) {
+ return new Content(EntityUtils.toByteArray(entity), ContentType.parse(entity.getContentType()));
+ } else {
+ return Content.NO_CONTENT;
+ }
+ }
+
+ /**
+ * Handles a successful response (2xx status code) and returns the response entity as a {@link Content} object.
+ * If no response entity exists, {@link Content#NO_CONTENT} is returned.
+ *
+ * @param response the HTTP response.
+ * @return a {@link Content} object that encapsulates the response body, or {@link Content#NO_CONTENT} if the
+ * response body is null or has zero length.
+ * @throws HttpResponseException if the response was unsuccessful (a >= 300 status code).
+ * @throws IOException if an I/O error occurs.
+ */
+ @Override
+ public Content handleResponse(final ClassicHttpResponse response) throws IOException {
+ final int statusCode = response.getCode();
+ final HttpEntity entity = response.getEntity();
+ final Content content = handleEntity(entity);
+ if (statusCode >= 300) {
+ final StringBuilder messageBuilder = new StringBuilder(response.getReasonPhrase());
Review Comment:
@arturobernalg I would not abuse the reason phase and would add an extra new attribute to the `HttpResponseException` class.
##########
httpclient5-fluent/src/main/java/org/apache/hc/client5/http/fluent/ContentResponseHandler.java:
##########
@@ -44,11 +46,63 @@
*/
public class ContentResponseHandler extends AbstractHttpClientResponseHandler<Content> {
+ /**
+ * Handles the response entity and transforms it into a {@link Content} object. If the response entity is null or
+ * has zero length, {@link Content#NO_CONTENT} is returned.
+ *
+ * @param entity the response entity.
+ * @return a {@link Content} object that encapsulates the response body, or {@link Content#NO_CONTENT} if the
+ * response body is null or has zero length.
+ * @throws IOException if an I/O error occurs.
+ */
@Override
public Content handleEntity(final HttpEntity entity) throws IOException {
- return entity != null ?
- new Content(EntityUtils.toByteArray(entity), ContentType.parse(entity.getContentType())) :
- Content.NO_CONTENT;
+ if (entity != null && entity.getContentLength() > 0) {
+ return new Content(EntityUtils.toByteArray(entity), ContentType.parse(entity.getContentType()));
+ } else {
+ return Content.NO_CONTENT;
+ }
+ }
+
+ /**
+ * Handles a successful response (2xx status code) and returns the response entity as a {@link Content} object.
+ * If no response entity exists, {@link Content#NO_CONTENT} is returned.
+ *
+ * @param response the HTTP response.
+ * @return a {@link Content} object that encapsulates the response body, or {@link Content#NO_CONTENT} if the
+ * response body is null or has zero length.
+ * @throws HttpResponseException if the response was unsuccessful (a >= 300 status code).
+ * @throws IOException if an I/O error occurs.
+ */
+ @Override
+ public Content handleResponse(final ClassicHttpResponse response) throws IOException {
+ final int statusCode = response.getCode();
+ final HttpEntity entity = response.getEntity();
+ final Content content = handleEntity(entity);
+ if (statusCode >= 300) {
+ final StringBuilder messageBuilder = new StringBuilder(response.getReasonPhrase());
+ if (content != null && content.getType() != null) {
+ messageBuilder.append(", body was ");
+ truncateString(content.asString(), messageBuilder);
Review Comment:
@arturobernalg The truncation logic looks flawed. What is the point of truncating the message if it has already been fully loaded in memory? Use `EntityUtils#toByteArray` with the content max limit instead.
##########
httpclient5-fluent/src/main/java/org/apache/hc/client5/http/fluent/ContentResponseHandler.java:
##########
@@ -44,11 +46,63 @@
*/
public class ContentResponseHandler extends AbstractHttpClientResponseHandler<Content> {
+ /**
+ * Handles the response entity and transforms it into a {@link Content} object. If the response entity is null or
+ * has zero length, {@link Content#NO_CONTENT} is returned.
+ *
+ * @param entity the response entity.
+ * @return a {@link Content} object that encapsulates the response body, or {@link Content#NO_CONTENT} if the
+ * response body is null or has zero length.
+ * @throws IOException if an I/O error occurs.
+ */
@Override
public Content handleEntity(final HttpEntity entity) throws IOException {
- return entity != null ?
- new Content(EntityUtils.toByteArray(entity), ContentType.parse(entity.getContentType())) :
- Content.NO_CONTENT;
+ if (entity != null && entity.getContentLength() > 0) {
Review Comment:
@arturobernalg Is this reasonable? It is legal for an entity to have its content length as -1 if it is chunk coded.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] arturobernalg commented on a diff in pull request #423: Enhances ContentResponseHandler to be fault-tolerant
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #423:
URL: https://github.com/apache/httpcomponents-client/pull/423#discussion_r1138907786
##########
httpclient5-fluent/src/main/java/org/apache/hc/client5/http/fluent/ContentResponseHandler.java:
##########
@@ -26,29 +26,57 @@
*/
package org.apache.hc.client5.http.fluent;
-import java.io.IOException;
-
+import org.apache.hc.client5.http.HttpResponseException;
import org.apache.hc.client5.http.impl.classic.AbstractHttpClientResponseHandler;
+import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.io.entity.EntityUtils;
+import java.io.IOException;
Review Comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c merged pull request #423: Enhances ContentResponseHandler to be fault-tolerant
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c merged PR #423:
URL: https://github.com/apache/httpcomponents-client/pull/423
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org