Skip to content

Conversation

@kyay10
Copy link
Collaborator

@kyay10 kyay10 commented Nov 29, 2025

The error suppression logic now matches resourceScope. Even though SagaBuilder is @PublishedApi, it was never used in any inline function, so it's technically private. For simplicity, I just deprecated it, but theoretically it could be removed

@kyay10 kyay10 requested review from nomisRev and serras November 29, 2025 20:33
@github-actions
Copy link
Contributor

github-actions bot commented Nov 29, 2025

Kover Report

File Coverage [57.89%]
arrow-libs/resilience/arrow-resilience/src/commonMain/kotlin/arrow/resilience/Saga.kt 57.89%
Total Project Coverage 47.54%

# Conflicts:
#	arrow-libs/resilience/arrow-resilience/build.gradle.kts
#	arrow-libs/resilience/arrow-resilience/src/commonMain/kotlin/arrow/resilience/Saga.kt
@kyay10 kyay10 requested a review from serras December 27, 2025 16:24
action: suspend SagaActionStep.() -> A,
compensation: suspend (A) -> Unit
): A =
guaranteeCase({ action(SagaActionStep) }) { res ->
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic of this change is:

  • the finalizer lambda doesn't call any coroutines code, so NonCancellable is pointless
  • when res is null, finalizer does nothing, so guaranteeCase simply catches and rethrows, which is pointless
  • when res is not null, guaranteeCase simply runs the action, and then the finalizer here (and again, NonCancellable is irrelevant)

Hence, guaranteeCase's code can be simplified to the following (assumptions: finalizer returns immediately when res is null, and finalizer doesn't suspend):

private suspend fun <A> guaranteeCase(
  fa: suspend () -> A,
  finalizer: suspend (value: A?) -> Unit
): A {
  val res = fa()
  finalizer(res)
  return res
}

which is just also!
There is a nasty bug in that implementation as well, which is that compensation doesn't get scheduled for null values! This fixes that as well.
Obviously, this is code left for binary compatibility, so I can roll this back, but it felt nicer just so that I can remove guaranteeCase and runReleaseAndRethrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants