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

Regular crash with "Not implemented" toast, app nearly unuseable #525

Open
edgarogh opened this issue Aug 29, 2024 · 3 comments
Open

Regular crash with "Not implemented" toast, app nearly unuseable #525

edgarogh opened this issue Aug 29, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@edgarogh
Copy link

edgarogh commented Aug 29, 2024

Describe the bug

When scrolling on my feed, the app sometimes encounters toots that it cannot parse. When it does, it just crashes with a toast that says "Not implemented". When this happens, I have no other choice but to wait a few days for the toot to go "out of sight".

As a comparison, Tusky doesn't crash, but deems the whole timeline unparseable and displays an error view in place of the whole timeline, that makes their app basically useless.

The crash is caused by a NPE here, where s.createdAt is actually null (see crash log below)

values.put("time", s.createdAt.getEpochSecond());

This value is null because the @RequiredField annotation was commented out by upstream Megalodon, for some dubious reason I cannot understand, since the possibility of a null value is apparently not handled

// @RequiredField // sometimes null on calckey
public Instant createdAt;

To reproduce

I have no idea how to reproduce it, it's probably specific to my instance. If you can somehow manually create a status object with a missing createdAt, it might work?

Possible solution

A better solution (compared to crashing or treating the whole timeline as invalid) would be to ignore unparseable toot objects, or deal with missing fields in a more error-tolerant way, or even have a special "cannot parse toot" view in the timeline with a button to view the toot online.

Does this happen in the official app?

No, because they didn't comment out the @RequiredField annotation like Megalodon does. However, it doesn't handle the error properly either and the whole timeline isn't rendered at all (like Tusky).

Version

Moshidon version: v2.3.0+fork.107.moshinda-play

Crash log

E AndroidRuntime: FATAL EXCEPTION: databaseThread
E AndroidRuntime: Process: org.joinmastodon.android.sk, PID: 20520
E AndroidRuntime: java.lang.NullPointerException: Attempt to invoke virtual method 'long j$.time.Instant.getEpochSecond()' on a null object reference
E AndroidRuntime: 	at org.joinmastodon.android.api.CacheController.lambda$putHomeTimeline$3(SourceFile:120)
E AndroidRuntime: 	at org.joinmastodon.android.api.CacheController.$r8$lambda$AJyW915yvcdDM3uSOc-JvH42ce8(SourceFile:0)
E AndroidRuntime: 	at org.joinmastodon.android.api.CacheController$$ExternalSyntheticLambda6.run(SourceFile:0)
E AndroidRuntime: 	at org.joinmastodon.android.api.CacheController.lambda$runOnDbThread$16(SourceFile:340)
E AndroidRuntime: 	at org.joinmastodon.android.api.CacheController.$r8$lambda$MQ0qlJB6DpKl0fkTnizgW1HSB6c(SourceFile:0)
E AndroidRuntime: 	at org.joinmastodon.android.api.CacheController$$ExternalSyntheticLambda10.run(SourceFile:0)
E AndroidRuntime: 	at android.os.Handler.handleCallback(Handler.java:959)
E AndroidRuntime: 	at android.os.Handler.dispatchMessage(Handler.java:100)
E AndroidRuntime: 	at android.os.Looper.loopOnce(Looper.java:232)
E AndroidRuntime: 	at android.os.Looper.loop(Looper.java:317)
E AndroidRuntime: 	at me.grishka.appkit.utils.WorkerThread.run(SourceFile:54)
@edgarogh edgarogh added the bug Something isn't working label Aug 29, 2024
@edgarogh
Copy link
Author

screenshot of the app with the annotation put back

Reverting the annotation removal is somewhat better than crashing, but it's in the same state as Mastodon: not usable at all.

@edgarogh
Copy link
Author

Ok so just adding an if statement around values.put("time", s.createdAt.getEpochSecond()); checking for nullity does seem to work perfectly fine, I might submit a PR but I'm still trying to understand if this would break something else. Also, it might be worth preemptively fixing other similar issues in the codebase.

@LucasGGamerM
Copy link
Owner

For some reason I feel like I have fixed this before, but idk. Thank you for the detailed info, it really helps out a lot 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants