I need to write a RestClient used by several application that return a specific object.
In case of bad request (400) I'd like to advice the caller application of the message error and status code.
I wonder if is it a good behavior to throw a managed Exception with code and message property in order to be catched properly from the caller code.
Something like this:
RestClient.java
ClientResponse response;
try {
response = client.resource(requestURI).queryParams(queryParams)
.type(MediaType.APPLICATION_JSON)
.accept(MediaType.APPLICATION_JSON)
.post(ClientResponse.class, richiesta);
boolean output = response.getStatus() == Response.Status.NO_CONTENT.getStatusCode();
if (!output && response.getStatus() == Response.Status.BAD_REQUEST)
throw new RestClientRuntimeException(response.getStatus, response.getEntity(String.class));
} catch (ClientHandlerException e) {
throw new RestClientRuntimeException(e);
} catch (UniformInterfaceException e) {
throw new RestClientRuntimeException(e);
}
Caller application
try
{
boolean output = restClient.method()
} catch (RestClientRuntimeException e) {
// read exception status and message entity
}
Is it a good practice ?
Your question has several aspects. Lets consider them separately:
Should I translate http error codes into exceptions?
Yes you should translate error codes into exceptions. Exceptions are nice (if used correctly of course). They separate the happy path from the exceptional cases. If you don't use exceptions but return codes, your callers need to check for that return code. And if you call several methods you get some nasty cascading ifs:
if (service1.method1() == NO_ERROR) {
if (service2.method2() = NO_ERROR) {
if (service3.method2() = NO_ERROR) {
...
} else {
...
}
} else {
...
}
} else {
...
}
Furthermore if you have several layers (and you almost certainly have), you need to do this at every level again. Exceptions are much better here. They let you code (and read!) the nice and clean happy path first and then
you can worry about the exceptions in the catch block. That's easier to write and easier to read.
Should I use checked exceptions?
This can get quite religious. Some people think checked exceptions are a good idea, some think, they are evil. I don't want to debate that here in detail. Just that: Do you want to force your callers to think about that particular exception? Is there always or at least in the vast majority of cases a way for the caller to handle that exception?
If you come to the conclusion that the caller cannot do anything but log the exception and fail itself, you should refrain from using checked exceptions. This makes the caller code much cleaner. There is no use in pointless try..catch blocks that are just there to make the compiler happy.
Speaking of REST: In case of a 500, there is most likely no chance to recover. If you get a 405 you most likely have a bug and there is no way to recover either. If you get a 409 Conflict with some special info on how to resolve the conflict there might be a good way to handle that (depending on your requirements). In such a case you may consider a checked exception.
Should I store the response code in the exception?
When you use generic RestClientRuntimeExceptions and have your callers query the response code, then your caller is obviously coupled to this being a REST call. You can do that if you are writing a generic REST client that can query arbitrary REST APIs. No problem in this case.
But you are already using a generic library (I guess Jersey). So what's the point in wrapping a generic API around a generic API? There may be some reasons (e.g. evaluating some internally used header params) but you should think about it whether this is justified.
Maybe you want to write an application-specific REST client, i.e. one that deserializes your application-specific representation classes (DTOs). In such a case you should not use generic exceptions but rather application-specific ones. If you use application-specific exceptions your callers are not coupled to this being a REST call. Maybe in the future a cool new technology shows up and you need to change your client. If you use the generic exceptions and have your callers evaluate the response codes, the callers need to change, too. But if you use application-specific exceptions, this interface will be more stable. The callers don't even need to know that such a thing as REST exists. This makes the caller code simpler and more maintainable as you separate concerns.
Should I put a rest client into a jar?
I know you haven't asked that question. But maybe you should do so. Providing a rest-client jar with a Jersey dependency to arbitrary clients (that's what it seems to me that you do) looks nice at first but can get you into real trouble with dependencies. If you are not convinced, I can explain that in detail but lets discuss this in a separate question.
Since your success response is JSON, I would model the error responses as JSON too.
For example consider the following JSON serialized from a POJO like ErrorMessage.java having corresponding fields.
{
"statusCode": 400,
"errorMessage": "Bad Request",
"errorDetails": "<details here>"
}
Since it is HTTP, it would be better to communicate error codes based on HTTP status codes. errorMessage and errorDetails can be blank in case of a successful status code.
Why don't you check the HTTP status code family?
Other errors besides 400 can happen. If you test the status code family, you catch them all:
switch (response.getStatusInfo().getFamily()) {
case CLIENT_ERROR:
// It's a client error
// You can throw an exception
break;
case SERVER_ERROR:
// It's a server error
// You can throw an exception
break;
default:
break;
}
IMHO it's a good practice at least for 400 status code to have custom exception because it implies malformed data was sent.
Instead of throwing unchecked exception and catching them, checked exceptions are more suitable.
