Tag: NPE

Take care when logging Exceptions!

Today I was facing some weird nullpointer exceptions (NPEs) in my Android App (Beta phase, luckily). Usually I catch exceptions like

try{ .. }catch(SomeException e){ 
   logger.info("A SomeException occured, but i got it.", e) 
}

Well in this part of the code I broke with my habit and wrote:

try{ .. }catch(SomeException e){ 
   logger.info("A SomeException occured, but i got it: "+e.getMessage(), e) 
}

And guess what. I experienced the expected exception, caught it (great) – and got a NPE somewhere in the Loggin framework. WTF?
After having a quick look at the Throwable API, I realized that getMessage() can indeed return null. And String+null produces null. So I nulled my logging message and passed this null reference right into the logging call – which produced the NPE in there. This was very annoying as I successfully caught the first exception – just to produce a susbsequent error during handling the first one.

Well, I immediately grepped my whole project for any strings like .getMessage() and checked for any other NPE traps.
Lesson learned today: carefully check the Api docs and be even more paranoid for NPEs.
Yet one question remains unanswered: Why on earth would one like to return null references in exceptions instead of empty strings?!

IllegalStateException: Content has been consumed

When working with Android or (to be more general) Apache HttpComponents, one should keep in mind that it depends on the HttpResponses (Api: Apache, Android) if they can be consumed multiple times or not. And if the response is not, then better triple check your code, what you’re doing.

I experiences the (very ugly) issue during the apparently simple task of reading the content of the response. )For ease of simplicity, I also use Apache commons-io, to read buffers etc.)

HttpResponse response = ...;
String content = IOUtils.toString(response.getEntity().entity.getContent());

Okay, how many possible Nullpointer Exceptions do you see? I didn’t care until I experienced the first one, so I made the code NPE safe:

HttpEntity entity = response.getEntity();
// entity can be null according to the docs!
if (entity != null) { 
    // The interface doesn't say that getContent() MUST return non-null
    InputStream stream = entity.getContent(); 
        if (stream != null) {
            tempContent = IOUtils.toString(entity.getContent());
        }
}

And suddenly I was faced with IllegalStateException: Content has been consumed. As I also did changes somewhere else, I assumed the error in some toString()-Methods that would read the content of the response during debugging. But as the error also showed up without debugging, I had a closer look to my “improvement”.

Well, the error was the call IOUtils.toString(entity.getContent()); which tried to re-aquire the Input Stream. But as I just aquired it two lines above for the null check, the content was already marked as consumed. So the (now hopefully) correct and robust code is:

HttpEntity entity = response.getEntity();
// entity can be null according to the docs!
if (entity != null) { 
    // The interface doesn't say that getContent() MUST return non-null
    InputStream stream = entity.getContent(); 
        if (stream != null) {
            tempContent = IOUtils.toString(<strong>stream</strong>);
        }
}

And the moral of the story

Be very carefull when reading HttpResponses! Also avoid pretty-printing the content in toString() – this might suddenly also consume your content. And good luck finding the point where you consume the content in such cases.

But .. why?! Please avoid returning null!

Yet I still wonder, why so many methods are allowed to return null instead of just an empty stream or something. All the Null-checks don’t make the code more readable. Some programmers might even be tempted to simply put an catch(NullPointerException e) around the part of 

response.getEntity().entity.getContent()

. Nothing I’d really recommend but I could also understand if I’d see something in other code.