Skip to content
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.

Sign up for GitHub

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

Jump to bottom

catch for EffectScope #2746

Merged
merged 19 commits into from Jul 21, 2022
Merged

catch for EffectScope #2746

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4f113bc
catch for EffectScope
serras Jun 23, 2022
822c89b
Update API files
serras Jun 23, 2022
39247b9
Merge branch 'main' into effect-catch
i-walker Jun 30, 2022
8c31fe3
Suggestions by @i-walker + add catch for EagerEffectScope
serras Jul 1, 2022
e9b86d2
Update API files
serras Jul 1, 2022
f18db85
Suggestions by @nomisRev to improve inference
serras Jul 6, 2022
533f0fc
Remove the intermediate `Attempt` class
serras Jul 6, 2022
4958b48
API dump
serras Jul 6, 2022
9693ab6
Merge branch 'main' into effect-catch
serras Jul 7, 2022
c19e720
Better docs based on @i-walker's suggestions
serras Jul 7, 2022
b77ce50
Add missing files + apply suggestions
serras Jul 8, 2022
c08c236
Fix Knit examples
serras Jul 8, 2022
ba8e4f3
Merge branch 'main' into effect-catch
nomisRev Jul 16, 2022
1e1f63e
Introduce forgotten suspend in catch for EffectScope
serras Jul 18, 2022
b1197af
Merge branch 'main' into effect-catch
serras Jul 18, 2022
93564cf
Update arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/co…
serras Jul 19, 2022
f3907a4
Remove suspend from EagerEffectScope.catch
serras Jul 19, 2022
81b0711
Add attempt/catch test for EagerEffect
serras Jul 20, 2022
08cf9e9
Merge branch 'main' into effect-catch
i-walker Jul 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions arrow-libs/core/arrow-core/api/arrow-core.api
View file
Open in desktop
Expand Up @@ -2694,6 +2694,7 @@ public final class arrow/core/continuations/EffectScope$DefaultImpls {
}

public final class arrow/core/continuations/EffectScopeKt {
public static final fun catch (Larrow/core/continuations/EffectScope;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function3;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
public static final fun ensureNotNull (Larrow/core/continuations/EffectScope;Ljava/lang/Object;Lkotlin/jvm/functions/Function0;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
}

Expand Down
14 changes: 14 additions & 0 deletions arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/continuations/EffectScope.kt
View file
Open in desktop
Expand Up @@ -240,3 +240,17 @@ public suspend fun <R, B : Any> EffectScope<R>.ensureNotNull(value: B?, shift: (
contract { returns() implies (value != null) }
return value ?: shift(shift())
}

/**
* Catch any possible `shift`ed outcome from [action], and transform it with [handler].
* This reads like a `catch` block, but for `EffectScope`.
*
* The [handler] may `shift` into a different `ErrorScope`, which is useful to
* simulate re-throwing of exceptions.
*/
public suspend fun<E, R, A> EffectScope<E>.catch(
Copy link
Member

Choose a reason for hiding this comment

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

Needs to be an extension due to variance?

This duplicates behaviour with handleErrorXXX but it exposes a nicer lambda for handler where you can leverage the DSL again by exposing EffectScope<E> on the receiver of the lambda.

I am absolutely up for improving these APIs, and renaming them but we should probably avoid duplicated APIs as much as possible to keep the API surface reasonable.

That being said this signatures reminds me a lot of Flow#catch, and it uses the same naming so I am in favour of catch over the current handleErrorXXX & redeemXXX methods. This seems to cover all 4 APIs with a single nice API.

Copy link
Member Author

Choose a reason for hiding this comment

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

For me the question is how much of the API should be exposed via EffectScope<E> as opposed to Effect. Personally, I'd prefer to stay within the realm of EffectScope as much as I could, and only refer to Effect to "run" the action at the very top level. But I also see this is very much a design choice, so I'll be happy with whatever decision you take.

Copy link
Member Author

Choose a reason for hiding this comment

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

About not being an extension, I just wrote it directly like that, although it could also be part of EffectScope itself. In fact, this opens the decades-old debate: it catch part of the "error effect", or is it a "effect transformer" (my definition of catch says that the latter).

Copy link
Member

@i-walker i-walker Jun 23, 2022 •
edited

Choose a reason for hiding this comment

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

This also an alternative, using the UnsafeVariance, which is fine here.

// in EffectScope interface

public suspend fun <B> Effect<R, B>.catch(recover: suspend EffectScope<R>.(@UnsafeVariance R) -> B): B =
    fold({ recover(it) }, ::identity)

we can also have the Effect be a parameter, if this is a better encoding

action: suspend EffectScope<R>.() -> A,
handler: suspend EffectScope<E>.(R) -> A
serras marked this conversation as resolved.
Show resolved Hide resolved
): A = effect(action).fold(
recover = { handler(it) }, transform = { x -> x }
)