Hi, I've noticed `Swipeable.animateTo` throw a `Jo...

# compose

[Daniele Segato]

Hi, I've noticed Swipeable.animateTo throw a JobCancellationException when it's done instead of just terminating. (actually ModalBottomSheetState.show() and ModalBottomSheetState.hide().

Is this a bug or was a design decision? And in the latter case what motivated it?

(more details in thread)

[Daniele Segato]

calling show()

suspend fun show() {
        val targetValue =
            if (isHalfExpandedEnabled) HalfExpanded
            else Expanded
        animateTo(targetValue = targetValue)
    }

uses Swipeable.animateTo

@ExperimentalMaterialApi
    suspend fun animateTo(targetValue: T, anim: AnimationSpec<Float> = animationSpec) {
        latestNonEmptyAnchorsFlow.collect { anchors ->
            try {
                val targetOffset = anchors.getOffset(targetValue)
                requireNotNull(targetOffset) {
                    "The target value must have an associated anchor."
                }
                animateInternalToOffset(targetOffset, anim)
            } finally {
                val endOffset = absoluteOffset.value
                val endValue = anchors
                    // fighting rounding error once again, anchor should be as close as 0.5 pixels
                    .filterKeys { anchorOffset -> abs(anchorOffset - endOffset) < 0.5f }
                    .values.firstOrNull() ?: currentValue
                currentValue = endValue
            }
        }
    }

the animateInternalToOffset is implemented like this:

private suspend fun animateInternalToOffset(target: Float, spec: AnimationSpec<Float>) {
        draggableState.drag {
            var prevValue = absoluteOffset.value
            animationTarget.value = target
            isAnimationRunning = true
            try {
                Animatable(prevValue).animateTo(target, spec) {
                    dragBy(this.value - prevValue)
                    prevValue = this.value
                }
            } finally {
                animationTarget.value = null
                isAnimationRunning = false
            }
        }
    }

And the drag call there is actually a DefaultDraggableState.drag for which this is the implementation

override suspend fun drag(
        dragPriority: MutatePriority,
        block: suspend DragScope.() -> Unit
    ): Unit = coroutineScope {
        scrollMutex.mutateWith(dragScope, dragPriority, block)
    }

now MutatorMutex.mutateWith is implemented like such

suspend fun <T, R> mutateWith(
        receiver: T,
        priority: MutatePriority = MutatePriority.Default,
        block: suspend T.() -> R
    ) = coroutineScope {
        val mutator = Mutator(priority, coroutineContext[Job]!!)

        tryMutateOrCancel(mutator)

        mutex.withLock {
            try {
                receiver.block()
            } finally {
                currentMutator.compareAndSet(mutator, null)
            }
        }
    }

and tryMutateOrCancel does exactly what it sais

private fun tryMutateOrCancel(mutator: Mutator) {
        while (true) {
            val oldMutator = currentMutator.get()
            if (oldMutator == null || mutator.canInterrupt(oldMutator)) {
                if (currentMutator.compareAndSet(oldMutator, mutator)) {
                    oldMutator?.cancel()
                    break
                }
            } else throw CancellationException("Current mutation had a higher priority")
        }
    }

call oldMutator?.cancel() in this case.

[Daniele Segato]

I'm not sure at which point in this sequence of operation but I think the cancel exception should be catched and handled like a terminal condition instead of being propagated above

[Ian Lake]

https://issuetracker.google.com/issues/200980998 is the canonical issue

[Ian Lake]

Accompanist Navigation Material's bottom sheet needed a lot of weird workarounds you might be interested in looking at e.g., https://github.com/google/accompanist/pull/742

[Daniele Segato]

Thanks!

[jossiwolf]

cc @matvei
@Daniele Segato, do you have a repro?

[Daniele Segato]

@jossiwolf every place i use a ModalBottomSheet reproduce it, should be easy to make one

[jossiwolf]

Thx! I tried reproing this recently and was only able to in certain scenarios, so this could be very helpful

[Daniele Segato]

I knew the issue was within swipeable are you saying it wasn't on/off issue and that knowing it always happens with show/hide on ModalBottomSheet make it reproducable and might help finding the root cause?

[Ian Lake]

I think what Jossi is saying is that if you have a simple, 100% repro case, you should attach it to that issuetracker issue I linked above