Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NullPointerException with double push #976

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

p1gp1g
Copy link

@p1gp1g p1gp1g commented Feb 6, 2024

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
Copy link
Author

p1gp1g commented Feb 6, 2024

@LucasGGamerM / @FineFindus this may interest you

@p1gp1g
Copy link
Author

p1gp1g commented Feb 6, 2024

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 mastodon/mastodon#27858

@FineFindus
Copy link

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?

@p1gp1g
Copy link
Author

p1gp1g commented Feb 7, 2024

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
Copy link
Author

p1gp1g commented Mar 28, 2024

@FineFindus should I open a PR on moshidon too ?

@FineFindus
Copy link

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 :)

@LucasGGamerM
Copy link

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

I am merging it

@LucasGGamerM
Copy link

LucasGGamerM commented Mar 29, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants