Android app Java code throws NullPointerException on production build despite null checks

Question

Crashlytics is reporting a lot of NullPointerExceptions in the following code in our Android app.

public static PayloadParseResult parsePayloadData(@NotNull String payloadStringData) {
    PayloadParseResult result = new PayloadParseResult();

    Gson gson = new GsonBuilder().create();
    NotificationPayload payload = gson.fromJson(payloadStringData, NotificationPayload.class);

    if (payload == null) {
        String log = "Unable to parse payload.\n";
        log += "payload string:\n" + payloadStringData;
        EventTracker.getsInstance().log(EventType.UnknownNotificationPayload, EventProperty.Type, log);

        result.isValid = false;
    } else if (payload.alertBody == null) {
        String log = "Unable to parse payload body.\n";
        log += "payload string:\n" + payloadStringData;
        EventTracker.getsInstance().log(EventType.UnknownNotificationPayload, EventProperty.Type, log);

        result.isValid = false;
    } else if (Strings.isNullOrEmpty(payload.alertBody.partitionType)) {
        String log = "Unknown partition type in payload. Should be 'team'.\n";
        log += "payload string:\n" + payloadStringData;
        EventTracker.getsInstance().log(EventType.UnknownNotificationPayload, EventProperty.Type, log);

        result.isValid = false;
    } else {
        // We understood this payload
        result.notificationPayload = payload;
        result.isValid = true;
    }

    return result;
}

The crash occurs on the last else if line.

} else if (Strings.isNullOrEmpty(payload.alertBody.partitionType)) {

The payload object is a simple POCO deserialized from json.

public class NotificationPayload {
    @SerializedName("a")
    public NotificationAlertBody alertBody;
}

public class NotificationAlertBody {
    @SerializedName("noteId")
    public String noteId;
    @SerializedName("partitionType")
    public String partitionType;
    @SerializedName("partitionKey")
    public String partitionKey;
}

I know what you are thinking. "There's no possible way for it to crash there." But it does. It has lowered our crash free user rate from 99.5% to 90.0% in a matter of days. We can't reproduce it in Android Studio, and we can't reproduce it in our own production testing. This is baffling.

Here is the call stack as shown in Crashlytics.

Fatal Exception: java.lang.NullPointerException
       at com.omitted.omitted.utils.gcm.PayloadParser.parsePayloadData(PayloadParser.java:87)
       at com.omitted.omitted.utils.gcm.PayloadParser.parsePayloadData(PayloadParser.java:66)
       at com.omitted.omitted.utils.gcm.MyCloudMessagingListenerService.onMessageReceived(MyCloudMessagingListenerService.java:32)
       at com.google.firebase.messaging.FirebaseMessagingService.dispatchMessage(FirebaseMessagingService.java:13)
       at com.google.firebase.messaging.FirebaseMessagingService.passMessageIntentToSdk(FirebaseMessagingService.java:8)
       at com.google.firebase.messaging.FirebaseMessagingService.handleMessageIntent(FirebaseMessagingService.java:3)
       at com.google.firebase.messaging.FirebaseMessagingService.handleIntent(FirebaseMessagingService.java:3)
       at com.google.firebase.messaging.EnhancedIntentService.lambda$processIntent$0$EnhancedIntentService(EnhancedIntentService.java:1)
       at com.google.firebase.messaging.EnhancedIntentService$$Lambda$0.run(EnhancedIntentService.java:6)
       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
       at com.google.android.gms.common.util.concurrent.zza.run(zza.java:6)
       at java.lang.Thread.run(Thread.java:923)

Edit:

I changed the crashing line to

} else if (payload.alertBody.partitionType == null || payload.alertBody.partitionType.isEmpty()) {

as @Shadov suggested, but I am still getting crash reports on that line.

No stacktrace and no reproducibility. Code where the issue can be reproduced [is a requirement](/help/mcve) — Zoe, Jul 17 '21 at 13:54
Maybe this is some faulty JIT optimization on the VM this is running, since you can't reproduce it 'in your own production'. I would change the `Strings.isNullOrEmpty` to `str == null || str.isEmpty()` - weird problem weird fix, seen similar things a couple times. — Shadov, Jul 17 '21 at 13:59
Are you 100% sure it's crashing on that exact line? If you use Proguard for your release builds, and you don't include `-keepattributes SourceFile,LineNumberTable` , then it will give an approximate line number location. — Mr-IDE, Jul 17 '21 at 14:01
@Zoe I get that is the rule, but I can't think of a better place for this problem. And this is most certainly not a duplicate of the associated question. — Joe Waller, Jul 17 '21 at 14:07
@Shadov Thank you for the suggestion. I guess the best I can do is change it as you suggested and see if the problem disappears. — Joe Waller, Jul 17 '21 at 14:09
[Don't post screenshots of errors](https://meta.stackoverflow.com/q/285551/6296561). Also doesn't help if you don't include the entire thing — Zoe, Jul 17 '21 at 14:12
@Zoe Considering everything is being checked for `null` in the method, up to the failure point of `string.isNullOrEmpty`, I don't think the duplicate you have linked is very helpful to solve this issue. Is there some way you know that the previous checks would fail, allowing `payload.alertBody` to be null at the point where the code tries to access `payload.alertBody.partitionType`? If not, the logical conclusion seems to be that the stack trace is extremely inaccurate and the exception is being thrown elsewhere. As someone with 23k reputation, could you shed any light on this? — ProgrammingLlama, Jul 17 '21 at 14:54
My best guess at this point is a parsing failure (GSON misbehaving), as you've indirectly pointed out, the line number being wrong. That could happen for a number of reasons though, including changes to the file that haven't been pushed to the stable channel, or the exception being from an old version that has the same effect. It's hard to tell without checking the data. I initially hammered as a cheap alternative to closing as no MCVE. I've reopened now. Been spending some time trying to track down `Strings.isNullOrEmpty`-related NPEs, but I can't find any. — Zoe, Jul 17 '21 at 14:59
@Zoe Yeah, I'd really hope that `Strings.isNullOrEmpty` can handle `null` values :D — ProgrammingLlama, Jul 17 '21 at 15:01
Dug into the source code of `Strings.isNullOrEmpty`, and if it's its fault, it's more likely down to an annotation processing error than anything else. It relies on `Platform.stringIsNullOrEmpty`, which is literally just `string == null || string.isEmpty()`. If a plain `string == null || string.isEmpty()` works, that's a substantial problem for guava, but one that can only be accounted for by a bad annotation. There [was a shift 3 months ago](https://github.com/google/guava/blame/master/guava/src/com/google/common/base/Strings.java) from @Nullable to @CheckForNull. — Zoe, Jul 17 '21 at 15:06
Either way, definitely a deeper problem, as proven by the full stacktrace. — Zoe, Jul 17 '21 at 15:06
Well, the only way I can think of is a concurrency issue (testing for `null` value and then getting an NPE because some field is set to `null` in the meantime). However, I don't see why a value would be set to `null` here... — MC Emperor, Jul 17 '21 at 19:18
Is this exception guaranteed to be thrown with the same input? Or is it more like _sometimes_? — MC Emperor, Jul 17 '21 at 19:20
I've looked for concurrency issues, but I'm not seeing anything that could remotely be problematic. With the help of a team member I have been able to reproduce it, but that helps us little as there is no obvious way to fix the problem. I'm about to wrap it up in a try-catch and be done with it. — Joe Waller, Jul 18 '21 at 12:02

Android app Java code throws NullPointerException on production build despite null checks

0 Answers0