megalodon icon indicating copy to clipboard operation
megalodon copied to clipboard

Fix NullPointerException with double push

Open p1gp1g opened this issue 1 year ago • 8 comments

I often experience a NullPointer Exception on double push:

java.lang.NullPointerException: Attempt to read from field 'org.joinmastodon.android.model.Account org.joinmastodon.android.model.Status.account' on a null object reference in method 'org.joinmastodon.android.model.PushNotification org.joinmastodon.android.model.PushNotification.fromNotification(android.content.Context, org.joinmastodon.android.model.Notification)'
	at org.joinmastodon.android.model.PushNotification.fromNotification(SourceFile:55)
	at org.joinmastodon.android.PushNotificationReceiver.notifyUnifiedPush(SourceFile:153)
	at org.joinmastodon.android.UnifiedPushNotificationReceiver$1.lambda$onSuccess$0(SourceFile:75)
	at org.joinmastodon.android.UnifiedPushNotificationReceiver$1.$r8$lambda$4J7AuBytjmt8335OwYDlx1gnhvU(SourceFile:0)
	at org.joinmastodon.android.UnifiedPushNotificationReceiver$1$$ExternalSyntheticLambda0.run(SourceFile:0)
	at android.os.Handler.handleCallback(Handler.java:958)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loopOnce(Looper.java:205)
	at android.os.Looper.loop(Looper.java:294)
	at me.grishka.appkit.utils.WorkerThread.run(SourceFile:54)

This fixes this issue.

Screenshot

p1gp1g avatar Feb 06 '24 08:02 p1gp1g

@LucasGGamerM / @FineFindus this may interest you

p1gp1g avatar Feb 06 '24 08:02 p1gp1g

BTW, from time to time (like, when you open the app, if the code bellow has not been called in the last 6h), it would be a good thing to do:

		if (!UnifiedPush.getDistributor(context).isEmpty()) {
			UnifiedPush.registerApp(
					context,
					session.getID(),
					getDEFAULT_FEATURES(),
					context.getPackageName()
			);
		}

There was an issue on mastodon preventing doing this, but this has been fixed since https://github.com/mastodon/mastodon/pull/27858

p1gp1g avatar Feb 06 '24 08:02 p1gp1g

The joys of writing Java code... Thanks for catching that.

BTW, from time to time (like when you open the app, if the code below has not been called in the last 6h), it would be a good thing to do:

I'm not quite sure why this is necessary. Could you please clarify?

FineFindus avatar Feb 07 '24 21:02 FineFindus

BTW, from time to time (like when you open the app, if the code below has not been called in the last 6h), it would be a good thing to do:

I'm not quite sure why this is necessary. Could you please clarify?

If somehow the server wipes the push endpoint, it will register it again.

For context, subscribing from time to time was not something we recommended for mastodon before the correction (27858 cited earlier), because mastodon didn't override previous subscriptions but appended it and it ended spamming push providers and adding load to the mastodon queue. When the patch has been released is a good example why registering from time to time is useful: some subscriptions (and push endpoints) were wiped with the upgrade. Registering again can be done when the app starts but I think adding a delay (x hours) is a good thing to avoid spamming the server.

p1gp1g avatar Feb 07 '24 22:02 p1gp1g

@FineFindus should I open a PR on moshidon too ?

p1gp1g avatar Mar 28 '24 18:03 p1gp1g

I had already mentioned this pr (alongside the other still open ones here) to @LucasGGamerM, but he probably just forgot about it, since there was and still is a lot of work going on with trying to merge all the new code by the official app. If you have the time, a PR would certainly be appreciated :)

FineFindus avatar Mar 28 '24 19:03 FineFindus

Yes. I completely forgot about this specific one. Silly me :D

I am merging it

LucasGGamerM avatar Mar 28 '24 19:03 LucasGGamerM

Yes. I completely forgot about this specific one. Silly me :D

I am merging it

I just merged this one and the one of the other notification related crashes PRs I found in Megalodon's repo. It's all on Moshidon now too :D

LucasGGamerM avatar Mar 29 '24 21:03 LucasGGamerM