I am working on a library in which I need to execute my URL using AsyncRestTemplate and after that I will get a json response back if it is successful. I am also making DataResponse
object whether the response is successful or not which I will return back to the caller of my library.
If the response is not successful then I need to log the error and make the DataResponse object appropriately depending on what type of error it is. I already have an enum DataErrorEnum
which contains most of the status code (HttpClientErrorException
and HttpServerErrorException
and some predefined) as shown below -
public enum DataErrorEnum {
OK(200, "NONE"),
BAD_REQUEST(400, "Server Bad Request"),
UNAUTHORIZED(401, " Server Unauthorized"),
SERVER_TIMEOUT(408, "Server Timeout"),
INTERNAL_SERVER_ERROR(500, "Internal Server Error"),
SERVERS_DOWN(6000, "Servers Down"),
ERROR_SERVICE(6002, "Random Error Occurred on Server");
// rest of the code
}
So I will be logging like this depending on what the error is since it will help to categorize how many were BAD_REQUEST
or UNAUTHORIZED
or any other status codes -
DataLogging.logErrors(ex, DataErrorEnum.BAD_REQUEST, keys); // if bad request
DataLogging.logErrors(ex, DataErrorEnum.UNAUTHORIZED, keys); // if unauthorized
DataLogging.logErrors(ex, DataErrorEnum.INTERNAL_SERVER_ERROR, keys); // if server error
DataLogging.logErrors(ex, DataErrorEnum.ERROR_SERVICE, keys); // if some random error occurred on server
And similarly I will return the DataResponse object as well for the error case like this -
new DataResponse(result.getBody(), DataErrorEnum.BAD_REQUEST, DataStatusEnum.ERROR)
new DataResponse(result.getBody(), DataErrorEnum.UNAUTHORIZED, DataStatusEnum.ERROR)
new DataResponse(result.getBody(), DataErrorEnum.INTERNAL_SERVER_ERROR, DataStatusEnum.ERROR)
new DataResponse(result.getBody(), DataErrorEnum.ERROR_SERVICE, DataStatusEnum.ERROR)
Below is the code I have so far which works fine but it is looking more messy in onFailure
method. Is there any way to make this more generic in onFailure
method? If I need to add couple more status code check, then I might need to add another if
else
block by which the code will grow very long and will not look good at all.
private AsyncRestTemplate restTemplate = new AsyncRestTemplate();
@Override
public Future<DataResponse> executeAsync(final DataKey keys) {
final SettableFuture<DataResponse> responseFuture = SettableFuture.create();
// for now I only have one machine in the list
List<String> hostnames = Arrays.asList("machineA");
executeForServers(responseFuture, keys, hostnames.get(0));
return responseFuture;
}
private void executeForServers(final SettableFuture<DataResponse> responseFuture, final DataKey keys,
final String hostName) {
restTemplate.exchange(generateURL(hostName, keys), HttpMethod.GET, keys.getEntity(), String.class).addCallback(
new ListenableFutureCallback<ResponseEntity<String>>() {
@Override
public void onSuccess(ResponseEntity<String> result) {
responseFuture.set(new DataResponse(result.getBody(), DataErrorEnum.OK,
DataStatusEnum.SUCCESS));
}
// can we simplify this by making more generic?
@Override
public void onFailure(Throwable ex) {
if (ex instanceof HttpClientErrorException) {
HttpClientErrorException httpException = (HttpClientErrorException) ex;
HttpStatus statusCode = httpException.getStatusCode();
if (statusCode == HttpStatus.BAD_REQUEST) {
DataLogging.logErrors(ex, DataErrorEnum.BAD_REQUEST, keys); // if bad request
responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
DataErrorEnum.BAD_REQUEST, DataStatusEnum.ERROR));
} else if (statusCode == HttpStatus.UNAUTHORIZED) {
DataLogging.logErrors(ex, DataErrorEnum.UNAUTHORIZED, keys); // if unauthorized
responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
DataErrorEnum.UNAUTHORIZED, DataStatusEnum.ERROR));
} else if (statusCode == HttpStatus.REQUEST_TIMEOUT) {
DataLogging.logErrors(ex, DataErrorEnum.SERVER_TIMEOUT, keys); // if server timeout
responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
DataErrorEnum.SERVER_TIMEOUT, DataStatusEnum.ERROR));
} else {
DataLogging.logErrors(ex, DataErrorEnum.ERROR_SERVICE, keys); // some random error on server
responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
DataErrorEnum.ERROR_SERVICE, DataStatusEnum.ERROR));
}
} else if (ex instanceof HttpServerErrorException) {
HttpServerErrorException httpException = (HttpServerErrorException) ex;
HttpStatus statusCode = httpException.getStatusCode();
if (statusCode == HttpStatus.INTERNAL_SERVER_ERROR) {
DataLogging.logErrors(ex, DataErrorEnum.SERVER_TIMEOUT, keys); // if server error
responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
DataErrorEnum.SERVER_TIMEOUT, DataStatusEnum.ERROR));
} else {
DataLogging.logErrors(ex, DataErrorEnum.ERROR_SERVICE, keys); // some random error on server
responseFuture.set(new DataResponse(httpException.getResponseBodyAsString(),
DataErrorEnum.ERROR_SERVICE, DataStatusEnum.ERROR));
}
} else if (ex instanceof RestClientException) {
DataLogging.logErrors(ex, DataErrorEnum.SERVERS_DOWN, keys); // server down
responseFuture.set(new DataResponse(null, DataErrorEnum.SERVERS_DOWN,
DataStatusEnum.ERROR));
}
}
});
}
Is there any way to simplify this messy code?