You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "ok2c (via GitHub)" <gi...@apache.org> on 2023/03/12 13:13:46 UTC
[GitHub] [httpcomponents-client] ok2c commented on a diff in pull request #423: Enhances ContentResponseHandler to be fault-tolerant
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