IllegalStateException: CommentViewHolder Two different ViewHolders have the same stable ID. Stable IDs in your adapter MUST BE unique and SHOULD NOT change.

zwarm commented 3 years ago

Sentry URL: https://sentry.io/share/issue/951ac7868ea54ac3bda4435aed19af05/

Similar to issue #14763 (Sentry Link) , but has a different stack trace.

To see both issues, this Sentry query works best.

I was not successful in reproducing this using Pixel 5 and WP.com or self-hosted site.

Stacktrace

``` java.lang.IllegalStateException: Two different ViewHolders have the same stable ID. Stable IDs in your adapter MUST BE unique and SHOULD NOT change. ViewHolder 1:CommentViewHolder{aba33ef position=2 id=890, oldPos=-1, pLpos:-1 not recyclable(1)} View Holder 2:CommentViewHolder{f4ca344 position=1 id=890, oldPos=-1, pLpos:-1} androidx.recyclerview.widget.RecyclerView{1a5d922 VFED..... ......ID 0,0-1080,1892 #7}, adapter:org.wordpress.android.ui.comments.CommentAdapter@9af300, layout:androidx.recyclerview.widget.LinearLayoutManager@a03a739, context:org.wordpress.android.ui.comments.CommentsActivity@c7ffaaf at androidx.recyclerview.widget.RecyclerView.handleMissingPreInfoForChangeError(RecyclerView.java:4268) at androidx.recyclerview.widget.RecyclerView.dispatchLayoutStep3(RecyclerView.java:4192) at androidx.recyclerview.widget.RecyclerView.dispatchLayout(RecyclerView.java:3862) at androidx.recyclerview.widget.RecyclerView.onLayout(RecyclerView.java:4404) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at android.widget.RelativeLayout.onLayout(RelativeLayout.java:1103) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at androidx.swiperefreshlayout.widget.SwipeRefreshLayout.onLayout(SwipeRefreshLayout.java:689) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at android.widget.RelativeLayout.onLayout(RelativeLayout.java:1103) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332) at android.widget.FrameLayout.onLayout(FrameLayout.java:270) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at androidx.recyclerview.widget.RecyclerView$LayoutManager.layoutDecoratedWithMargins(RecyclerView.java:9587) at androidx.recyclerview.widget.LinearLayoutManager.layoutChunk(LinearLayoutManager.java:1685) at androidx.recyclerview.widget.LinearLayoutManager.fill(LinearLayoutManager.java:1587) at androidx.recyclerview.widget.LinearLayoutManager.onLayoutChildren(LinearLayoutManager.java:665) at androidx.recyclerview.widget.RecyclerView.dispatchLayoutStep2(RecyclerView.java:4134) at androidx.recyclerview.widget.RecyclerView.dispatchLayout(RecyclerView.java:3851) at androidx.recyclerview.widget.RecyclerView.onLayout(RecyclerView.java:4404) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at androidx.viewpager2.widget.ViewPager2.onLayout(ViewPager2.java:527) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at com.google.android.material.appbar.HeaderScrollingViewBehavior.layoutChild(HeaderScrollingViewBehavior.java:148) at com.google.android.material.appbar.ViewOffsetBehavior.onLayoutChild(ViewOffsetBehavior.java:43) at com.google.android.material.appbar.AppBarLayout$ScrollingViewBehavior.onLayoutChild(AppBarLayout.java:1996) at androidx.coordinatorlayout.widget.CoordinatorLayout.onLayout(CoordinatorLayout.java:918) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332) at android.widget.FrameLayout.onLayout(FrameLayout.java:270) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332) at android.widget.FrameLayout.onLayout(FrameLayout.java:270) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332) at android.widget.FrameLayout.onLayout(FrameLayout.java:270) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1829) at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1673) at android.widget.LinearLayout.onLayout(LinearLayout.java:1582) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332) at android.widget.FrameLayout.onLayout(FrameLayout.java:270) at com.android.internal.policy.DecorView.onLayout(DecorView.java:820) at android.view.View.layout(View.java:23042) at android.view.ViewGroup.layout(ViewGroup.java:6419) at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:3786) at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3227) at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2182) at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8730) at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1352) at android.view.Choreographer.doCallbacks(Choreographer.java:1149) at android.view.Choreographer.doFrame(Choreographer.java:1049) at android.view.Choreographer$FrameHandler.handleMessage(Choreographer.java:1275) at android.os.Handler.dispatchMessage(Handler.java:106) at android.os.Looper.loop(Looper.java:233) at android.app.ActivityThread.main(ActivityThread.java:8010) at java.lang.reflect.Method.invoke(Method.java) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:631) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:978) ```

zwarm commented 3 years ago

@AliSoftware - Not sure I captured all the details that I need to capture. Can you help out if I missed anything. Thanks

AliSoftware commented 3 years ago

@zwarm I see that the Sentry issue is still linked to the other GitHub Issue #14763:

🖼 Screenshot

![image](https://user-images.githubusercontent.com/216089/129019268-9e9a675a-4545-40dd-9490-5f0f9e77ce73.png)

Which is actually why I reopened the other Issue in the first place, thinking at first that the (new?) event was already linked to the (old) Issue and that they were the same (I didn't take a close enough look at the stacktraces back then).

I have to admit didn't look too much in details at how the Sentry events were separate or merged or whatnot, but maybe if they indeed end up having the same linked GH Issue, that could be because those 2 events got merged by Sentry (into a single issue that points to the sameold Issue) at some point while they shouldn't have… which led to this same GH link? Or maybe they never got merged and have always been separate issues but the new one was just linked to the wrong (old) GitHub Issue by mistake instead of this new one? 🤷

In any case, the linked GH issue in the new Sentry issue probably needs to be fixed to point to that current issue instead 😉

(PS: If it helps, we have a script in our mobile-toolbox to create those GH issues like this one from Sentry issues, automatically with all the relevant information and adding the GH issue link back too)

AliSoftware commented 3 years ago

Ok actually I think the Sentry events did indeed get merged into a single issue, since the share link that is in the other GH issue #14763 – https://sentry.io/share/issue/75d4493fe7f5459ea340929e3fc6aecf/ – points to the same Sentry issue https://sentry.io/organizations/a8c/issues/2325012745/ that you linked in this current GH issue, they both link to events of the issue ID 2325012745 in Sentry that shows as last appearing in 17.7. Hence my confusion ☝️ in the first place and why they are both linked to the same GitHub issue, since they are the same Sentry issue.

Since the various events in that Sentry issue have different stacktrace as you pointed out, and should thus be considered different issues, we should probably unmerge them in Sentry to keep them as separate Sentry issues with different GitHub issue links and different resolution statuses / tags / stats.

zwarm commented 3 years ago

So without having a degree in Sentry, I was not able to unmerge the issues. I did attach the sentry query above that shows the events for both issues. I also linked this GH to the Sentry ticket. The numbers in sentry seem to be all over the place, but the numbers don't appear to be that high. I will be keeping an eye on this throughout my rotation and if the numbers spike, then I'll change the priority, I am setting the priority to low.

sentry-io[bot] commented 3 years ago

Sentry issue: WORDPRESS-ANDROID-1DFX

develric commented 3 years ago

Reporting here a few notes we were discussing on slack for traceability.

Unfortunately I was not able to reproduce the issue as well, but I'm noting that CommentViewHolder and CommentSubHeaderViewHolder were refactored in 18.0 beta to UnifiedCommentViewHolder and UnifiedCommentSubHeaderViewHolder ; with this refactor some effort was put to avoid comments duplication from cache, that could hopefully help with the issue.

I would go like this if other folks agree:

monitor the 18.0 beta for occurrences of the issue (even if heads up we should look for UnifiedCommentViewHolder and UnifiedCommentSubHeaderViewHolder in sentry). In my understanding those 2 classes does not seem to appear in sentry events, but we are still under the lower beta numbers of users, so worth continue monitoring.
If the numbers moves to the new classes we can further dig the issue on the new classes, otherwise…(I would not say more for pure superstition 🙃…🤞)

cc @AliSoftware and @zwarm

sentry-io[bot] commented 5 months ago

Sentry Issue: JETPACK-ANDROID-81P

wordpress-mobile / WordPress-Android

IllegalStateException: CommentViewHolder Two different ViewHolders have the same stable ID. Stable IDs in your adapter MUST BE unique and SHOULD NOT change. #15161