added error json response for broker endpoints #302
Conversation
| @@ -440,7 +440,7 @@ trait BrokerCli { | |||
|
|
|||
| val json = | |||
| try { sendRequestObj[HttpLogResponse]("/broker/log", params) } | |||
| catch { case e: IOException => throw new Error("" + e) } | |||
| catch { case e: IOException =>throw new Error("" + e)} | |||
| if (connection.getResponseCode != 200) throw new IOException(connection.getResponseCode + " - " + connection.getResponseMessage) | ||
| case e: IOException => { | ||
| if (connection.getResponseCode != 200) { | ||
| val br = new BufferedReader(new InputStreamReader(connection.getErrorStream)) |
There was a problem hiding this comment.
you can use guava's CharStreams.toString method here instead of rolling your own.
| @@ -68,7 +68,7 @@ trait BrokerApiComponentImpl extends BrokerApiComponent { | |||
| Response.ok(BrokerStatusResponse(brokerNodes)) | |||
| .build() | |||
| case Failure(e) => | |||
| Response.status(Response.Status.BAD_REQUEST).build() | |||
| Response.status(Response.Status.BAD_REQUEST).entity(HttpErrorResponse(400, e.getMessage())).build() | |||
There was a problem hiding this comment.
Can you just refactor Status.BadRequest(...) to do this instead of copy/pasting it to each place it's used?
|
Can you comment on what this is trying to fix/improve? |
|
Apologies for the late reply. This commit changes the error responses within the broker endpoints to return JSON values. Currently we have a service hitting these endpoints and when something goes wrong (i.e attempting to remove a broker which is currently running) HTML is returned. It is more helpful to return JSON so that our service can interpret the response and react appropriately. |
|
Cool, seems good to me. Have you tested the backwards-compat case with an old client -> new server (and vis versa)? |
| if (connection.getResponseCode != 200) throw new IOException(connection.getResponseCode + " - " + connection.getResponseMessage) | ||
| case e: IOException => { | ||
| if (connection.getResponseCode != 200) { | ||
| val errorMessage:String = CharStreams.toString(new InputStreamReader(connection.getErrorStream())) |
There was a problem hiding this comment.
you should be able to omit the type annotation here.
| @@ -68,7 +68,7 @@ trait BrokerApiComponentImpl extends BrokerApiComponent { | |||
| Response.ok(BrokerStatusResponse(brokerNodes)) | |||
| .build() | |||
| case Failure(e) => | |||
| Response.status(Response.Status.BAD_REQUEST).build() | |||
| return Status.BadRequest(e.getMessage) | |||
There was a problem hiding this comment.
omit return keyword here.
No description provided.