Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

findCollidingIds AssertionError exception keeps crashing the app #4

Closed
DarkionAvey opened this issue Dec 24, 2018 · 7 comments
Closed

Comments

@DarkionAvey
Copy link

DarkionAvey commented Dec 24, 2018

Hi!
I've been using the app for couple of days now, and I must say I'm impressed, well done! But I noticed that the app crashes when scrolling very far down the list. According to logcat, it's caused by SubredditSubmissionsAdapter#findCollidingIds.

I didn't go through the code so I just silenced the exception in a fork. For now I haven't faced any side effects, but have a look at the issue and possibly fix it when you have time.

Thanks

@Tunous
Copy link
Contributor

Tunous commented Dec 24, 2018

I would like to try and fix this too.
@saket is there a way for us to know the reason why you added said method in this commit:
c6b6114 ? The link added in comment doesn't work for me.

@saket
Copy link
Owner

saket commented Dec 25, 2018

findCollidingIds() throws an error if two SubmissionRowUiModel items in the data-set have the same IDs. In this case, the collision is happening between items of SubredditSubmission.

The adapterId is generated here: JrawUtils2#L20. Fortunately or unfortunately this function is used for generating the adapter ID of multiple types of Reddit related objects.

I'm using a combination of hashcode + created-at-time in hopes that they'll be unique, but clearly that's wrong. Multiple submissions can have the same created-at timestamp, increasing their likelihood of collision.

@Tunous unfortunately, the Bugsnag dashboard is private so you won't be able to open the link. It doesn't have anything useful except for links to the two colliding submissions. I wonder if I can make my bugsnag dashboard public.

@DarkionAvey
Copy link
Author

But if you read the printed logcat (using Log.e), SubmissionRowUiModel 'existing' and 'row' are basically the same entry, so the issue isn't in findCollidingIds but somewhere upstream where an article is retrieved twice (?)

@saket
Copy link
Owner

saket commented Dec 26, 2018

You're right. The rows point to the same submission. Looks like the problem lies somewhere else upstream.

Tunous added a commit to Tunous/Dawn that referenced this issue Dec 26, 2018
@Tunous
Copy link
Contributor

Tunous commented Dec 26, 2018

I've debugged the bug a bit and found that the problem lies in part of code which deals with saving new submissions to local database:

The code inside of the linked if statement executes basically every time because saveTimeMillis is a part of primary key for CachedSubmissionId2. Since everything (including duplicates) fetched from the API is saved to the database we are getting id collisions.

I've fixed this bug by removing saveTimeMillis from the primary key for CachedSubmissionId2 and adding id here instead which seems to be something that is supposed to uniquely identify submissions.


@saket are you interested in accepting pull requests to this repository or would you prefer to leave everything up to forks?

@saket
Copy link
Owner

saket commented Dec 29, 2018

@Tunous just an information about the bug fix is sufficient, don't bother sending back patches to Dank. As much as I'd like them, I don't think I have enough time to maintain this app anymore.

saket pushed a commit that referenced this issue Jan 12, 2019
@saket
Copy link
Owner

saket commented Jan 13, 2019

Will be fixed with #6

@saket saket closed this as completed Jan 13, 2019
saket pushed a commit that referenced this issue Jan 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants