patrykandpatrick / vico

A light and extensible chart library for Android.
https://patrykandpatrick.com/vico/wiki
Apache License 2.0
2.08k stars 125 forks source link

Reading a state that was created after the snapshot was taken or in a snapshot that has not yet been applied #651

Closed boswelja closed 5 months ago

boswelja commented 5 months ago

How to reproduce

I'm not sure on the exact steps to reproduce, as I have many different graph types and this only happens with the one. Please see this PR for my Vico graph setup. The issue only happens with TemperatureGraph. If I remove that from UI, everything works fine. https://github.com/boswelja/NASdroid/blob/f431fba573f91760822f0500b44676d5efc40c69/features/reporting/ui/src/main/kotlin/com/nasdroid/reporting/ui/overview/Graphs.kt#L218

Observed behavior

App crashes with the following error

17:18:14.691  E  FATAL EXCEPTION: DefaultDispatcher-worker-6
                 Process: com.nasdroid, PID: 2255
                 java.lang.IllegalStateException: Reading a state that was created after the snapshot was taken or in a snapshot that has not yet been applied
                    at androidx.compose.runtime.snapshots.SnapshotKt.readError(Snapshot.kt:2033)
                    at androidx.compose.runtime.snapshots.SnapshotKt.readable(Snapshot.kt:2017)
                    at androidx.compose.runtime.SnapshotMutableStateImpl.getValue(SnapshotState.kt:135)
                    at com.patrykandpatrick.vico.compose.state.CartesianChartModelWrapperState.getValue(CartesianChartModelWrapper.kt:56)
                    at com.patrykandpatrick.vico.compose.model.ChartEntryModelExtensionsKt$collectAsState$1$startAnimation$1.invoke(ChartEntryModelExtensions.kt:69)
                    at com.patrykandpatrick.vico.compose.model.ChartEntryModelExtensionsKt$collectAsState$1$startAnimation$1.invoke(ChartEntryModelExtensions.kt:67)
                    at com.patrykandpatrick.vico.core.model.CartesianChartModelProducer$UpdateReceiver.handleUpdate(CartesianChartModelProducer.kt:230)
                    at com.patrykandpatrick.vico.core.model.CartesianChartModelProducer.registerForUpdates(CartesianChartModelProducer.kt:146)
                    at com.patrykandpatrick.vico.compose.model.ChartEntryModelExtensionsKt$collectAsState$1$1.invokeSuspend(ChartEntryModelExtensions.kt:108)
                    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
                    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
                    at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:585)
                    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:802)
                    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:706)
                    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:693)
                    Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [androidx.compose.ui.platform.MotionDurationScaleImpl@87d7149, androidx.compose.runtime.BroadcastFrameClock@d63aa4e, StandaloneCoroutine{Cancelling}@7a7776f, Dispatchers.Default]

Expected behavior

App does not crash, and the corresponding graph is displayed.

Vico version(s)

Latest unstable version

Android version(s)

Android 14

Additional information

The project currently uses Compose 1.7 alpha, but I'm able to reproduce this even with Compose 1.6 stable. Please let me know if I'm doing anything wrong :smil

patrickmichalik commented 5 months ago

Hello! Thanks for the report. To the best of my knowledge, this is a Compose bug—see issue #625. Just to confirm, when testing this with Compose 1.6, did you downgrade all Compose libraries? androidx.compose.material:material-icons-extended, for example, should also be downgraded—see issue #407. If so, could you please share a list of the versions to which you downgraded the libraries? The relevant section of the libs.versions.toml file is perfectly sufficient.

I’d be happy to test this myself, but I don’t have a server to which I could connect the app. If there’s any way in which I could see the charts on my end and reproduce the issue, please let me know.

boswelja commented 5 months ago

Thanks for the quick reply! I'm fairly sure I got all the dependencies, but I can try again this weekend. I can also try set up a preview to reproduce it, if that helps?

patrickmichalik commented 5 months ago

That would be perfect, @boswelja. Any runnable example affected by the issue will suffice. Thanks!

boswelja commented 5 months ago

Just a quick update, I'm not sure what's changed, but I can no longer reproduce this with just a single graph. I'm wondering if there's too much work happening somewhere and it's causing some kind of race condition. Will keep you posted! Edit: I dropped the data points way down, and I'm still getting the same error. I'll set up a preview with 3 graphs and see what happens Edit 2: It's not an issue in previews, the best kind of error 😒

boswelja commented 5 months ago

Alright, I figured it out

It's a combination of LazyVerticalGrid and having one or more graphs that are over a certain complexity. If the graphs are not in a LazyVerticalGrid, then the issue does not happen. If there aren't enough graphs, or all of the graphs are super simple, then the issue does not happen.

I also did some dependency analysis and found the rogue dependency that was bringing Compose up to 1.7 alpha. It's not an issue on 1.6 stable. I'll close this issue because of that. Thanks for your help!

patrickmichalik commented 4 months ago

Thanks for the update, @boswelja! I’m glad to hear that you’ve identified the problematic dependency and that the issue doesn’t occur with Compose 1.6.

We’ll update Vico to Compose 1.7 once a stable version of Compose 1.7 is released and included in the Compose BOM. Before publishing a Vico release that depends on Compose 1.7, we’ll do some extra testing, attempt to reproduce the exception in question, and make any changes that may be required for compatibility.

Since this turned out not to be a bug on our end, I’ll be using “Close as not planned.” Should the problem reoccur with a stable Compose 1.7 release and a Vico version that depends on it, please feel free to reopen the issue. Cheers!