debugjavaMinor
Varnish - HttpClient Exception Handling
Viewed 0 times
httpclienthandlingvarnishexception
Problem
I'm consuming some messages from a rabbit queue. Within the consumer/processor I need to invalidate a varnish cache and requeue the message if the purge failed.
I'm using a local purge proxy.
The consumer is a Dropwizard microservice but I'm using a BasicHttpClient because JerseyClient doesn't allow me (AFAIK) to set PURGE as http verb.
I'm trying to improve my error handling.. Is throwing an IllegalStateException fine or should I improve it?
Thanks
This is the consumer..
```
public MessageResult apply(UpdateMessage t) {
if( !agentValidator.validMessage( t ) ){
return MessageResult.ACK; // discar
I'm using a local purge proxy.
The consumer is a Dropwizard microservice but I'm using a BasicHttpClient because JerseyClient doesn't allow me (AFAIK) to set PURGE as http verb.
I'm trying to improve my error handling.. Is throwing an IllegalStateException fine or should I improve it?
Thanks
public void invalidateVarnishCache( String level, String idUrl ) throws IOException {
try{
String xPurgeRegex = level+"/"+idUrl+"$";
Header header = new BasicHeader( "X-Purge-Regex", xPurgeRegex );
BasicHttpRequest purgeRequest = new BasicHttpRequest("PURGE", "/" );
purgeRequest.setHeader(header);
LOGGER.log( LogEntries.VarnPurge, level, idUrl, host, xPurgeRegex );
response = httpClient.execute(host, purgeRequest);
entity = response.getEntity();
int statusCode = response.getStatusLine().getStatusCode();
if( !is2XXResponse( statusCode ) ){
int respLength = entity.getContent().available();
byte[] errorResp = new byte[ respLength ];
entity.getContent().read(errorResp, 0, respLength);
LOGGER.log( LogEntries.VarnPurgeErrResp, statusCode, new String(errorResp) );
throw new IllegalStateException("Varnish local proxy returned response with status code "+statusCode+" from Varnish backend. Consumed message will be requeued.");
/////////////////////
}else{
LOGGER.log( LogEntries.VarnPurgeSuccessResp, statusCode );
}
EntityUtils.consume(entity);
}catch( IOException ex ){
LOGGER.log( LogEntries.VarnPurgeException, ex );
throw ex;
}
}This is the consumer..
```
public MessageResult apply(UpdateMessage t) {
if( !agentValidator.validMessage( t ) ){
return MessageResult.ACK; // discar
Solution
The Consumer
The nested try-catch blocks look weird and useless. Since
By the way, catching of
invalidateVarnishCache
The only purpose of the try-catch block in the original code is to trace the
Concerning the
I don't know what is the range of response statuses that this sort of proxy can return, but your implementation does not make distinction between 4xx and 5xx codes, which may lead either to a client-side error (your request is invalid, 4xx) or to a server-side error (5xx), which is semantically close to an I/O exception. For the latter, I can suggest to wrap the response message into a
The nested try-catch blocks look weird and useless. Since
purgeVarnishBeforePcsRequest and getLatestStateOfResource throw exceptions, they may be caught in the outer catch block.try {
pcsConnector.purgeVarnishBeforePcsRequest( level, idUrl );
response = connector.getLatestStateOfResource( urlRequest );
// .....
return MessageResult.ACK;
} catch (Exception e) {
// logger
return MessageResult.NACK_REQUEUE;
}By the way, catching of
Exception type might be a bad coding practice, but I'd not discuss it here because the context is not enough to judge.invalidateVarnishCache
The only purpose of the try-catch block in the original code is to trace the
IOException using a hypothetical commented logger and then re-throw it. I doubt that the block itself is useful, because exactly the same exception is re-thrown and must be handled somewhere among the callers and even probably also logged by them. Wouldn't it be redundant?Concerning the
IllegalStateException, the problem is that it is a RuntimeException and the caller can miss to handle it properly, with hardly predictable consequences.I don't know what is the range of response statuses that this sort of proxy can return, but your implementation does not make distinction between 4xx and 5xx codes, which may lead either to a client-side error (your request is invalid, 4xx) or to a server-side error (5xx), which is semantically close to an I/O exception. For the latter, I can suggest to wrap the response message into a
IOexception and re-throw it. For the former, it's still up to you to decide how to handle it, probably just logging is enough.Code Snippets
try {
pcsConnector.purgeVarnishBeforePcsRequest( level, idUrl );
response = connector.getLatestStateOfResource( urlRequest );
// .....
return MessageResult.ACK;
} catch (Exception e) {
// logger
return MessageResult.NACK_REQUEUE;
}Context
StackExchange Code Review Q#126769, answer score: 2
Revisions (0)
No revisions yet.