arkivanov / Essenty

The most essential libraries for Kotlin Multiplatform development
Apache License 2.0
495 stars 15 forks source link

[StateKeeper] Add another ```register``` method with onError parameter #86

Closed DatL4g closed 1 year ago

DatL4g commented 1 year ago

The current statekeeper implementation is pretty nice, espacially when using Decompose, however I think this feature should be supported.

Currently the StateKeeper has this method only

fun <T : Parcelable> register(key: String, supplier: () -> T?)

An additional (optional) onError method would be handy

fun <T : Parcelable> register(key: String, supplier: () -> T?, onError: (T?, Throwable) -> Unit)

To handle the exception and saving yourself.

E.g. some (Android) devices could throw a TooLargeException, so we could save it in a file for example as fallback

arkivanov commented 1 year ago

I believe TransactionTooLargeException can not be intercepted, because it happens internally when the Bundle is serialized.

Also I don't understand how is the linked issue https://github.com/DATL4G/BurningSeries-Android/issues/20 related? E.g. Compose saves the scrolling positions automatically. With normal Android views the UI should save the scrolling position, e.g. via StateKeeper. How is it related to error handling?

DatL4g commented 1 year ago

Should be possible to wrap the saving method by try - catch / runCatching.

It's not completly related to https://github.com/DATL4G/BurningSeries-Android/issues/20 but I save a pretty large state and putting the app in background throws an Exception. The App crashes and the scroll state is lost

arkivanov commented 1 year ago

Should be possible to wrap the saving method by try - catch / runCatching.

I don't see any place where this could be done. StateKeeper just builds a Bundle with data, at this point there are no errors thrown. The Bundle is then given to Android where it's serialized, and where errors may happen. See the code.

The following code is basically equivalent:

    private lateinit var stateKeeper: StateKeeperDispatcher

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        stateKeeper  = StateKeeperDispatcher(savedState = savedInstanceState?.getParcelable(KEY_SAVED_STATE))
    }

    override fun onSaveInstanceState(outState: Bundle, outPersistentState: PersistableBundle) {
        super.onSaveInstanceState(outState, outPersistentState)

        // No errors thrown here
        outState.putParcelable(KEY_SAVED_STATE, stateKeeper.save())
    }

    private companion object {
        private const val KEY_SAVED_STATE = "saved_state"
    }
}

So an error may be thrown internally once onSaveInstanceState returned.

DatL4g commented 1 year ago

Previously I did something like this

override fun onSaveInstanceState(): Parcelable {
    val state = try {
        super.onSaveInstanceState()
    } catch (ignored: Exception) {
        BaseSavedState.EMPTY_STATE
    }
    val save = try {
        SaveState(
            data1,
            data2
        )
    } catch (ignored: Exception) { state }

    return save ?: BaseSavedState.EMPTY_STATE
}
arkivanov commented 1 year ago

This looks like saving state in an Android View, I believe TransactionTooLargeException may not be thrown there. Perhaps some other view-specific errors are possible though. But this looks unrelated to using StateKeeper for Activity/Fragment state preservation. There are no errors thrown when calling StateKeeper#save, unless there is a bug in the application.

DatL4g commented 1 year ago

Okay seems like there is not really a way to catch that. But maybe you could provide the bundle or something like a check method (instead of onError) so we can check the size of the bundle

arkivanov commented 1 year ago

Perhaps it is already possible!

First of all, add the following function to calculate the size of Parcelable.

fun Parcelable.getSizeInBytes(): Int {
    val parcel = Parcel.obtain()
    try {
        parcel.writeParcelable(this, 0)
        return parcel.marshall().size
    } finally {
        parcel.recycle()
    }
}

Then there are at least two options.

  1. Check the size of the Bundle in onSaveInstanceState as follows:
override fun onSaveInstanceState(outState: Bundle) {
    val bundle = Bundle()
    super.onSaveInstanceState(bundle)

    if (bundle.getSizeInBytes() <= SAVED_STATE_MAX_SIZE) {
        outState.putAll(bundle)
    } else {
        // Do something else
    }
}
  1. Create StateKeeper in Activity or Fragment manually as follows:
private const val KEY_STATE = "STATE_KEEPER_STATE"
private const val SAVED_STATE_MAX_SIZE = 500_000_000

fun SavedStateRegistryOwner.stateKeeper(onBundleTooLarge: () -> Unit = {}): StateKeeper {
    val dispatcher = StateKeeperDispatcher(savedStateRegistry.consumeRestoredStateForKey(KEY_STATE)?.getParcelable(KEY_STATE))

    savedStateRegistry.registerSavedStateProvider(KEY_STATE) {
        val savedState = dispatcher.save()
        val bundle = Bundle()

        if (savedState.getSizeInBytes() <= SAVED_STATE_MAX_SIZE) {
            bundle.putParcelable(KEY_STATE, savedState)
        } else {
            onBundleTooLarge()
        }

        bundle
    }

    return dispatcher
}

Would this work for you?

DatL4g commented 1 year ago

I'm using decompose so the statekeeper is already created, I'm not creating it on my own.

But I could just switch to a singleton to save the state as it's so big

arkivanov commented 1 year ago

You can still create StateKeeper on your own, you will also need to create the root ComponentContext manually:

DefaultRootComponent(
    componentContext = DefaultComponentContext(
        lifecycle = essentyLifecycle(),
        stateKeeper = stateKeeper(onBundleTooLarge = { ... }),
        instanceKeeper = instanceKeeper(),
        backHandler = backHandler(),
    ),
)
DatL4g commented 1 year ago

This works thanks. The max size is 500_000 btw.

The getParcelable(key: String) is deprecated and getParcelable(key: String, clazz: Clazz<T>) should be used. Is it possible to use that method?

arkivanov commented 1 year ago

Ah sorry, it's 500_000 indeed.

Yes, it should be possible to use the new getParcelable method, but only on API>=33. See the docs - https://developer.android.com/reference/android/os/Bundle#getParcelable(java.lang.String,%20java.lang.Class%3CT%3E)

DatL4g commented 1 year ago

There seems to be a bug in the new getParcelable method that may throw a NullPointerException. I will leave this little code snippet if anyone else needs it

@Suppress("DEPRECATION")
inline fun <reified T : Parcelable> Bundle.getCompatParcelable(key: String): T? {
    return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
        val result = runCatching {
            this.getParcelable(key, T::class.java)
        }
        if (result.isFailure) {
            this.getParcelable(key)
        } else {
            result.getOrNull()
        }
    } else {
        this.getParcelable(key)
    }
}
arkivanov commented 1 year ago

Thanks! Closing the issue for now.