search

wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.97k stars 1.32k forks source link

Post list fails to refresh or crashes the app when you scroll quickly #14860

Closed antonis closed 3 years ago

antonis commented 3 years ago

Expected behavior

The posts list refreshes in all cases

Actual behavior

The post list fails to refresh when you scroll down too quickly (sometimes causing a crash) or after cleaning the cache

Unable to refresh https://user-images.githubusercontent.com/304044/122415971-0372e100-cf91-11eb-8c51-00b0e55de301.mp4
Crash log ``` java.lang.IllegalArgumentException: Illegal start(60) or end(80) position for totalSize(62) at org.wordpress.android.fluxc.model.list.datasource.InternalPagedListDataSource.getItemIds(InternalPagedListDataSource.kt:61) at org.wordpress.android.fluxc.model.list.datasource.InternalPagedListDataSource.getItemsInRange(InternalPagedListDataSource.kt:51) at org.wordpress.android.fluxc.model.list.PagedListPositionalDataSource.loadRangeInternal(PagedListFactory.kt:58) at org.wordpress.android.fluxc.model.list.PagedListPositionalDataSource.loadRange(PagedListFactory.kt:49) at androidx.paging.PositionalDataSource.loadRange(PositionalDataSource.kt:430) at androidx.paging.PositionalDataSource.load$paging_common(PositionalDataSource.kt:348) at androidx.paging.LegacyPagingSource$load$2.invokeSuspend(LegacyPagingSource.kt:116) at androidx.paging.LegacyPagingSource$load$2.invoke(Unknown Source:10) at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(Undispatched.kt:91) at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(Builders.common.kt:165) at kotlinx.coroutines.BuildersKt.withContext(Unknown Source:1) at androidx.paging.LegacyPagingSource.load(LegacyPagingSource.kt:115) at androidx.paging.LegacyPageFetcher$scheduleLoad$1.invokeSuspend(LegacyPageFetcher.kt:53) at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) at java.lang.Thread.run(Thread.java:920) ```

This seems to be new to 17.6 according to Sentry

Steps to reproduce the behavior

By scrolling

  1. Go to the post list and scroll down quickly
  2. Notice that the list fails to refresh or the app crashes
  3. Notice that the list is empty (and it shouldn't)
  4. Try to refresh by pulling down
  5. Notice that the screen does not refresh
  6. Go back to the My Sites screen
  7. Open the post list again
  8. Notice the the posts are there

By clearing the cache (not reproduced consistently)

  1. Go to the app settings and clear the cache and stored data
  2. Open the app and login again
  3. Go to the post list
  4. Notice that the list is empty (and it shouldn't)
  5. Try to refresh by pulling down
  6. Notice that the screen does not refresh
  7. Go back to the My Sites screen
  8. Open the post list again
  9. Notice the the posts are there
Tested on Pixel 2 XL / 5, Android 11 / 12 beta 2, WPAndroid 17.6-rc-1
thehenrybyrd commented 3 years ago

I was able to reproduce this by scrolling quickly, but also when opening the app for the first time after a couple days.

Scrolling quickly:

https://user-images.githubusercontent.com/15107387/123232370-c1dabc80-d4e1-11eb-9dd5-906603714ab1.mp4

Opening the app for the first time in a bit:

https://user-images.githubusercontent.com/15107387/123232480-dfa82180-d4e1-11eb-841d-e804976dccc3.mov

I was also able to reproduce this by clearing cache and stored data, then restarting my phone.

It's particularly scary opening the app for the first time in a while, as it has that "I lost all my posts!" sensation.

Samsung Galaxy S21, Android 11, WPAndroid 17.6-rc-3
pachlava commented 3 years ago

Reproduced the You haven't published any posts yet situation by scrolling down through a large number of blog posts. Not attaching any video since I don't have a proper test website with enough posts in it, and used one of the A8C P2s to reproduce this.

Samsung A31, Android 10, WPAndroid 17.6.-rc-3

ashiagr commented 3 years ago

Looks like it started happening after migrating to Paging3 library v3.0.0 for the new unified comments list here: https://github.com/wordpress-mobile/WordPress-Android/commit/9095c66e5fedac1bd5567cfb554e257fca4e2009 in #14709.

As per this migration guide

Paging 3 is significantly different from earlier versions of the Paging library.

Posts screen uses Paging2 (PagedListAdpter), it is possible that Paging2 is not working well alongside this new version even though this Android ticket mentions:

The new version includes all methods and classes from the old, just deprecated.

I can start migrating Posts screen to use Paging 3, but it might not be straightforward considering Paging3 is almost a rewrite of Paging2. @khaykov, keeping you in the loop 🙂 as I might need your help with this.

ashiagr commented 3 years ago

I'll also try a workaround around this part in FluxC tomorrow with a fresh mind for a possible beta fix.

ParaskP7 commented 3 years ago

👋 @antonis @thehenrybyrd !

I was also able to reproduce the You haven't published any posts yet both:

I used the AFK P2 site to get a large amount of blog posts to scroll through.

Tested on:

Google Pixel 4, Android S (12), WPAndroid 17.6.-rc-3

khaykov commented 3 years ago

@ashiagr Sure, I'll be happy to help! I totally missed that the Post list is using Paging 2 🤦