Retrofit2: How to properly check the validity of response.body()?

Question

Since version retrofit2.retrofit:2.3.0 I'm getting NullPointerException warnings on response.body() even when checking the body for null before:

Method invocation 'getCar()' may produce 'java.lang.NullPointerException'

In the changelog for 2.3.0 was an entry related to null checks:

Retrofit now uses @Nullable to annotate all possibly-null values. [...] We use @ParametersAreNonnullByDefault and all parameters and return types are never null unless explicitly annotated @Nullable.

It that the intended behavior? In my point of view response.body() should be immutable, so my check in Picture 1 should not show a warning.

This is not a question about NullPointerExceptions - it's about the way how to handle responses of Retrofit2 properly. To not have warnings at the moment I have to do something like this:

if(response != null) {
    CarResponseBody body = response.body();
    if (body != null && body.getCar() != null){
        CarResponse car = body.getCar();
    }
}

A lot of code to just check if there is a valid response...

Answer 1

Is that the intended behavior?

If you check the JavaDoc of Response<T> you can read

@Nullable public T body() The deserialized response body of a successful response.
Javadoc: Response

As it suggests, body() won't be null if the response was successful. To check for success you have isSuccessful() which

Returns true if code() is in the range [200..300).

So the @Nullable is a valid choice, since the response can be null in any non-success case, e.g. no network, an invalid request, or some other error.

The hint in your IDE is a lint warning that checks your source code for possible sources of common errors or bugs.

This is why body() might be null and why lint reports this as a warning in the first place.

In my point of view response.body() should be immutable, so my check in Picture 1 should not show a warning.

And in theory you're correct. You and I both know that body() is not mutable, but the problem here is that this lint check has no way of knowing.

T res1 = response.body(); // could be null
T res2 = response.body(); // ...maybe still null?

Lint is a static source and byte code analyzer that helps to prevent common bugs and errors and one of those lint checks tries to prevent NPEs. If you annotate a method @Nullable all the check knows is that the returned value could be null and it will raise a warning if you try to operate on the result of the call directly.

// does response return the same value twice? who knows?
response.body() != null && response.body().getCar() != null

The way you handle your response is actually the only way to get rid of the lint warning—other than suppressing or disabling it.

By assigning it to a local variable you are able to ensure that a value is not null at some point and that it won't become null in the future, and lint is also able to see that.

CarResponseBody body = response.body(); // assign body() to a local variable
if (body != null && body.getCar() != null) { // variable is not null
    CarResponse car = body.getCar(); // safe to access body, car is also not null
    // everything is fine!
}

---

It that the intended behavior?

Yes. @Nullable is a good way to hint that a method might return null and you should also use it in your own code if you return null on some paths because lint can warn of possible NullPointerExceptions.

If a method might return null you will have to assign it to a local field and check the field for null values, or you risk that the value could have changed.

Car car = response.getBody(); // car could be null
if(car != null) {
  // car will never be null
}

Different options / approaches

I see that you also additionally seem to wrap your response object in an additional layer.

CarResponseBody body = response.body();
Car car = body.getCar()

If you want to remove complexity from your code, you should have a look at how to remove this wrapping *ResponseBody at an earlier stage. You can do so by registering your own Converter and adding additional handling there. You can see more about this on this Retrofit talk by Jake Wharton

Another completely different approach would be to use RxJava with Retrofit which removes the need to check the response yourself. You'll either get a success or an error, which you can handle the Rx way.

Answer 2

I usually create an isValid() or hasResults() method in the WhateverResponse class

boolean isValid(){
return getCar() != null && getWheel() != null && somethingElse
}

usage

if(response.isValid()){
//do your other if's
}

checking for null response isn't necessary. errorBody() will be not null or the retrofit2.Callback onFailure() method will be invoked before that happens