-
Notifications
You must be signed in to change notification settings - Fork 463
kyay10/saga-as-resource #3794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
kyay10/saga-as-resource #3794
Conversation
Kover Report
|
b1fd85a to
6106954
Compare
# Conflicts: # arrow-libs/resilience/arrow-resilience/build.gradle.kts # arrow-libs/resilience/arrow-resilience/src/commonMain/kotlin/arrow/resilience/Saga.kt
| action: suspend SagaActionStep.() -> A, | ||
| compensation: suspend (A) -> Unit | ||
| ): A = | ||
| guaranteeCase({ action(SagaActionStep) }) { res -> |
There was a problem hiding this comment.
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
finalizerlambda doesn't call any coroutines code, soNonCancellableis pointless - when
resis null,finalizerdoes nothing, soguaranteeCasesimply catches and rethrows, which is pointless - when
resis not null,guaranteeCasesimply runs theaction, and then the finalizer here (and again,NonCancellableis 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
The error suppression logic now matches
resourceScope. Even thoughSagaBuilderis@PublishedApi, it was never used in anyinlinefunction, so it's technicallyprivate. For simplicity, I just deprecated it, but theoretically it could be removed