svenjacobs / reveal

Reveal effect (coach mark) for Compose Multiplatform targeting Android, iOS, Desktop and Web
MIT License
431 stars 7 forks source link

java.io.NotSerializableException: com.svenjacobs.reveal.RevealCanvasState #89

Closed lehmansnDev closed 7 months ago

lehmansnDev commented 10 months ago

Describe the bug The exception only occurs rarely. So far it has only occurred when starting the app. Unfortunately, I can't say much more about the problem. If you other information, please feel free to ask.

Expected behavior

FATAL EXCEPTION: main
    Process: de.lehmansn.birthdays, PID: 29604
    android.os.BadParcelableException: Parcelable encountered IOException writing serializable object (name = [First Screen])
        at android.os.Parcel.writeSerializable(Parcel.java:2797)
        at android.os.Parcel.writeValue(Parcel.java:2563)
        at android.os.Parcel.writeValue(Parcel.java:2362)
        at android.os.Parcel.writeList(Parcel.java:1415)
        at android.os.Parcel.writeValue(Parcel.java:2506)
        at android.os.Parcel.writeValue(Parcel.java:2362)
        at android.os.Parcel.writeList(Parcel.java:1415)
        at android.os.Parcel.writeValue(Parcel.java:2506)
        at android.os.Parcel.writeValue(Parcel.java:2362)
        at android.os.Parcel.writeArrayMapInternal(Parcel.java:1298)
        at android.os.BaseBundle.writeToParcelInner(BaseBundle.java:1843)
        at android.os.Bundle.writeToParcel(Bundle.java:1389)
        at android.os.Parcel.writeBundle(Parcel.java:1367)
        at android.os.Parcel.writeValue(Parcel.java:2479)
        at android.os.Parcel.writeValue(Parcel.java:2369)
        at android.os.Parcel.writeArrayMapInternal(Parcel.java:1298)
        at android.os.BaseBundle.writeToParcelInner(BaseBundle.java:1843)
        at android.os.Bundle.writeToParcel(Bundle.java:1389)
        at android.os.Parcel.writeBundle(Parcel.java:1367)
        at android.os.Parcel.writeValue(Parcel.java:2479)
        at android.os.Parcel.writeValue(Parcel.java:2369)
        at android.os.BaseBundle.dumpStats(BaseBundle.java:1917)
        at android.os.BaseBundle.dumpStats(BaseBundle.java:1954)
        at android.app.servertransaction.PendingTransactionActions$StopInfo.collectBundleStates(PendingTransactionActions.java:123)
        at android.app.servertransaction.PendingTransactionActions$StopInfo.run(PendingTransactionActions.java:139)
        at android.os.Handler.handleCallback(Handler.java:958)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loopOnce(Looper.java:205)
        at android.os.Looper.loop(Looper.java:294)
        at android.app.ActivityThread.main(ActivityThread.java:8177)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:971)
    Caused by: java.io.NotSerializableException: com.svenjacobs.reveal.RevealCanvasState
        at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1240)
        at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1620)
        at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1581)
        at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1490)
        at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1234)
        at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:354)
        at android.os.Parcel.writeSerializable(Parcel.java:2792)
        at android.os.Parcel.writeValue(Parcel.java:2563) 
        at android.os.Parcel.writeValue(Parcel.java:2362) 
        at android.os.Parcel.writeList(Parcel.java:1415) 
        at android.os.Parcel.writeValue(Parcel.java:2506) 
        at android.os.Parcel.writeValue(Parcel.java:2362) 
        at android.os.Parcel.writeList(Parcel.java:1415) 
        at android.os.Parcel.writeValue(Parcel.java:2506) 
        at android.os.Parcel.writeValue(Parcel.java:2362) 
        at android.os.Parcel.writeArrayMapInternal(Parcel.java:1298) 
        at android.os.BaseBundle.writeToParcelInner(BaseBundle.java:1843) 
        at android.os.Bundle.writeToParcel(Bundle.java:1389) 
        at android.os.Parcel.writeBundle(Parcel.java:1367) 
        at android.os.Parcel.writeValue(Parcel.java:2479) 
        at android.os.Parcel.writeValue(Parcel.java:2369) 
        at android.os.Parcel.writeArrayMapInternal(Parcel.java:1298) 
        at android.os.BaseBundle.writeToParcelInner(BaseBundle.java:1843) 
        at android.os.Bundle.writeToParcel(Bundle.java:1389) 
        at android.os.Parcel.writeBundle(Parcel.java:1367) 
        at android.os.Parcel.writeValue(Parcel.java:2479) 
        at android.os.Parcel.writeValue(Parcel.java:2369) 
        at android.os.BaseBundle.dumpStats(BaseBundle.java:1917) 
        at android.os.BaseBundle.dumpStats(BaseBundle.java:1954) 
        at android.app.servertransaction.PendingTransactionActions$StopInfo.collectBundleStates(PendingTransactionActions.java:123) 
        at android.app.servertransaction.PendingTransactionActions$StopInfo.run(PendingTransactionActions.java:139) 
        at android.os.Handler.handleCallback(Handler.java:958) 
        at android.os.Handler.dispatchMessage(Handler.java:99) 
        at android.os.Looper.loopOnce(Looper.java:205) 
        at android.os.Looper.loop(Looper.java:294) 
        at android.app.ActivityThread.main(ActivityThread.java:8177) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:971) 

Smartphone (please complete the following information):

lehmansnDev commented 10 months ago

I have analyzed it further. It happens when my smartphone is locked and I start the app from my IDE. The deployment is finished and I am unlocking my smartphone. Then the exception is thrown. It's not that bad, it's not a real scenario for users.

svenjacobs commented 10 months ago

That's interesting, thanks @lehmansnDev. Are you somehow putting a RevealCanvasState in a rememberSaveable? Because that is actually not supported.

lehmansnDev commented 10 months ago

Okay, good to know. But I don't put it into a rememberSavable. It is just a parameter of the screen. Maybe it is because I am using Voyager.

setContent {
        val revealCanvasState = rememberRevealCanvasState()
        val birthdaysScreen = rememberScreen(SharedScreen.Birthdays(revealCanvasState))

        BirthdaysTheme {
            RevealCanvas(
                modifier = Modifier.fillMaxSize(),
                revealCanvasState = revealCanvasState,
            ) {
                Navigator(birthdaysScreen) { navigator ->
                    SlideTransition(navigator)
                }
            }
        }
}

I would close the issue, because it is not really an issue for me. Is that fine for you?

svenjacobs commented 10 months ago

I will close this ticket because it seems to not affect apps in production.

lehmansnDev commented 9 months ago

Hey Sven, The error occurs more often than I thought. Whenever I put the app in the background, the exception is thrown. It's due to Voyager, it only allows parameters that can be serialized. I think RevealCanvasState is not serializable, so the app crashes at that moment. Is it possible to make RevealCanvasState serializable?

svenjacobs commented 9 months ago

@lehmansnDev I wonder how Voyager can force RevealCanvasState to be serialized when we only use remember and not rememberSaveable? Anyway, since RevealCanvasState contains a lambda I need to check if it's even possible to serialize this class. Is there no way in Voyager to exclude a class from serialization? Could you maybe post some example code?

svenjacobs commented 7 months ago

Hello @lehmansnDev, did you see my questions above?

lehmansnDev commented 7 months ago

Hey @svenjacobs, sorry, I totally forgot about the message. I think it's because a Screen can be serialized in Voyager

public actual interface Screen : Serializable {
    public actual val key: ScreenKey
        get() = commonKeyGeneration()

    @Composable
    public actual fun Content()
}

and if its parameters are not serializable, then saving the state probably no longer works. But that's just a guess...

I have made a small example project with a multi-module navigation. reveal-voyager-test If you now click on the hardware home button in the SecondScreen, the app crashes and you have to restart the app. The navigation itself works. Does this help you or do you need more information?

svenjacobs commented 7 months ago

Okay, now I understand. You're passing RevealCanvasState into the constructor of a Screen implementation. So this is not strictly happening in a Compose context.

Please see the solution here. Can you try putting a @Transient to the RevealCanvasState argument?

However since the value is not restored by serialization, you might have the problem that Reveal isn't working anymore after the Voyager state was restored. You could store the instance of RevealCanvasState outside of Compose as a global variable and use it as a default value for the argument or ditch the constructor argument and straightforwardly use the global var. But I would have to open the constructor of the class because currently it is internal only.

svenjacobs commented 7 months ago

I just released version 3.0.5 where I opened the constructors. It make take a while until it's available on Maven Central. Please check if the proposed solution above is working for you.

svenjacobs commented 7 months ago

I tried it out with your test application and the proposed solution works, please see here. Note that I'm not familiar with Voyager and there might be a better solution for this kind of problem.

lehmansnDev commented 7 months ago

Nice, thank you @svenjacobs. I like your solution 👍 For me, the issue can be closed 🙂

svenjacobs commented 7 months ago

Great, closing it! 🙂