In terms of DRY (don't repeat yourself), I often feel that Spring adding the StatusCode
in front of a HttpStatusCodeException
is somehow counter-intuitive. And especially is against DRY, because the statuscode is anyway offered as http status code in the response.
Thus, I want to suggest the following test should succeed:
@Test
public void test() {
assertEquals("junit", new HttpStatusCodeException(HttpStatusCode.FORBIDDEN, "junit"); //fails, because message = "403 junit"
}
Because as of now, if the HttpStatusCodeException is thrown from a @RestController
, Spring renders a message like:
{
...
"message": "403 junit"
...
}
Source:
HttpStatusCodeException.class
:
private static String getMessage(HttpStatusCode statusCode, String statusText) {
if (!StringUtils.hasLength(statusText) && statusCode instanceof HttpStatus status) {
statusText = status.getReasonPhrase();
}
return statusCode.value() + " " + statusText; //suggestion: remove the statusCode prefix here
}
``
**Comment From: snicoll**
I am not sure what the argument for DRY is. This is an exception that carry a status code, of course its message should output the status code.
> Thus, I want to suggest the following test should succeed:
Sorry, but I really see no reason that this should be. This completely ignores the first argument of the exception.
**Comment From: membersound**
Okay, but which exception then has to be thrown in Spring that creates a certain http statuscode in the response, and only carries the custom message? Without prefixing the message?
**Comment From: snicoll**
> which exception then has to be thrown in Spring
I am not sure what you're asking. We surely don't provide every exceptions with every possible use cases you may fancy. If you want an exception to behave like that, create your own.
> Without prefixing the message?
It isn't the message. it is the `statusText` (please review the Javadoc).
**Comment From: membersound**
Excuse me - I just noticed I wrote HttpStatusCodeException, but I ment: `HttpClientErrorException` (in case that matters).
Well let's say I want to throw some exceptions based on conditions, inside a RestController. Isn't `HttpClientErrorException` the correct exception to use?
If so, I would want to simply show the message to the user *without* prefixing the status code before my custom message. And that's not possible!
@RestController public class MyController { @GetMapping("/test") public void test() { throw new HttpClientErrorException(HttpStatusCode.TOO_MANY_REQUESTS, "custom message"); } }
Result:
{ "message": "429 custom message" } - with statuscode=429 in the response itself ```
Well and this is exactly misleading, as only the custom message should be present in the response body, because the 429 is already included in the httpcode alongside with the response. And it comes from HttpStatusCodeException.getMessage()
as this prefixes the "statusText" with the httpcode.
And I still think that exposing the http status code twice (1st as statuscode in the response, and 2nd inside the message body) contradicts any DRY principle.
Comment From: bclozel
HttpClientErrorException is an exception thrown by HTTP clients. Please ask a question on StackOverflow.
Comment From: membersound
Oh, I then probably should switch to ResponseStatusException
.