You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/03/15 06:50:04 UTC

[GitHub] [pulsar] nodece commented on a change in pull request #14677: Fixed 404 error msg not being returned correctly using http lookup.

nodece commented on a change in pull request #14677:
URL: https://github.com/apache/pulsar/pull/14677#discussion_r826628560



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/web/RestException.java
##########
@@ -49,7 +49,11 @@ public RestException(Response.Status status, String message) {
     }
 
     public RestException(int code, String message) {
-        super(message, Response.status(code).entity(new ErrorData(message)).type(MediaType.APPLICATION_JSON).build());
+        super(message, Response
+                .status(code, message)

Review comment:
       I agreed with @RobertIndie, you can improve the following code in  https://github.com/apache/pulsar/blob/d02bd7378acd1f202d8bcf50aa6999749e61b20b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/HttpClient.java#L210:
   ```java
   if (response2.getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) {
     e = new NotFoundException("Not found: " + (StringUtils.isNotEmpty(response2.getResponseBody()) ?
                                               response2.getResponseBody() : response2.getStatusText()));
   } else {
       e = new PulsarClientException("HTTP get request failed: " + (StringUtils.isNotEmpty(response2.getResponseBody()) ?
                                               response2.getResponseBody() : response2.getStatusText()));
    }
   ```
   
   or you 
   

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/web/RestException.java
##########
@@ -49,7 +49,11 @@ public RestException(Response.Status status, String message) {
     }
 
     public RestException(int code, String message) {
-        super(message, Response.status(code).entity(new ErrorData(message)).type(MediaType.APPLICATION_JSON).build());
+        super(message, Response
+                .status(code, message)

Review comment:
       I agreed with @RobertIndie, you can improve the following code in  https://github.com/apache/pulsar/blob/d02bd7378acd1f202d8bcf50aa6999749e61b20b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/HttpClient.java#L210:
   ```java
   if (response2.getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) {
     e = new NotFoundException("Not found: " + (StringUtils.isNotEmpty(response2.getResponseBody()) ?
                                               response2.getResponseBody() : response2.getStatusText()));
   } else {
       e = new PulsarClientException("HTTP get request failed: " + (StringUtils.isNotEmpty(response2.getResponseBody()) ?
                                               response2.getResponseBody() : response2.getStatusText()));
    }
   ```
   




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org