https://kotlinlang.org logo
Join Slack
Powered by
As part of my library <Pedestal>, I have an `Eithe...
# arrow-contributors
c
As part of my library Pedestal, I have an 
Either
variant that supports unfinished operations using a three-state sealed class. How could interoperability with the 
Raise
DSL look like?
s
With context receivers it would be much easier though.. Just add a 
Raise
per failure subcase.
Copy code
inline fun <E, A> outcome(block: (Raise<Incomplete>, Raise<Failure<E>>) () -> A): ProgressiveOutcome<E, A> = ...
Looking the code, it would be easiest to add a common type between 
Incomplete
and 
Failure
and put that into 
Raise<E>
y
Perhaps you could have a 
Raise<Error>
, and a 
Raise<Progress>
with contexts Dang, Simon beat me to it, although I'm suggesting to make sure the type is unwrapped
c
I see, so the Quiver team considers that any non-finished value is a raise-able failure
our types are different though, a 
ProgressiveOutcome.Success
can be in-progress
s
I see, so the Quiver team considers that any non-finished value is a raise-able failure
Uhm, no 🤔 I wrote that implementation, but did a dirty trick to allow naturally binding 
Either<E, A>
. It's a pretty neat trick 😄 Allow raising 
Any?
, and have the public API nicely typed.
👀 1
Hmm, let me give it a shot for your data type.
c
I mean, it's not really the code I'm worried about, it's "whether it even makes sense"
but since the Quiver team did implement it, I guess it does, so I'll give it a shot
s
Here you go. Not sure if this is entirely what you're looking for, and it's untested 😅
Untitled.cpp
Doing 
Either<E, A>.bind()
on 
Left
should now show-up as 
ProgressiveOutcome.Failure
which is pretty cool
y
These unsafe casts personally scare me though. I can see issues here if e.g. someone tries to raise a 
Failure
. The code also doesn't handle 
Incomplete
on the recover side which I'm guessing was a simple oversight, but it kinda shows how easily the code can do unexpected things. Perhaps what would be better is 3 context receivers: 
Raise<Progress>
for incomplete, 
Raise<Pair<E, Progress>>
for Failure, and a 
Raise<E>
, which is just the failure raise 
.withError({ it to done() }) { ... }
s
The casting is completely hidden from the end API, so if anything goes wrong it's a bug tbh
Your solution allows for controlling the progress, I didn't do that but I would personally deal with the different. You could add 
progress
state to 
OutcomeRaise
.
Copy code
val progress: Int
fun updateProgress(block: (Int) -> Int): Unit
That way you can control the progress imperatively from the DSL, since it seems that incomplete or failure is 
done
by default. Some strategy can even be applied here, which is accepted as param in 
outcome { }
.
So not entirely I follow your comment @Youssef Shoaib [MOD].
I can see issues here if e.g. someone tries to raise a 
Failure
. The code also doesn't handle 
Incomplete
on the recover side which I'm guessing was a simple oversight, but it kinda shows how easily the code can do unexpected things.
Can you give an example?
y
Well firstly 
Incomplete(42).bind()
results in a 
Failure(Incomplete(42), Int.MAX_VALUE)
Secondly, with a 
OutcomeRaise<Outcome<F, S>>
, calling 
raise(myFailure)
where 
myFailure: Failure<F>
will fail with a CCE when later examined because it'll "flatten out" the failure i.e. it'll result in 
myFailure
when the intention was 
Failure(myFailure, Int.MAX_VALUE)
s
Okay, that's a bug in my impl somewhere then 😅 Let me check
c
Sorry for taking so long to reply, I was trying to do it through the super-type solution. https://gitlab.com/opensavvy/groundwork/pedestal/-/compare/arrow-2...progressive-raise?from_project_id=37325377&amp;straight=false
Uh, but of course, my version doesn't allow 
raise(5)
🤔
y
I don't think the 2nd issue with 
raise(myFailure)
can be fixed without a bunch of code gymnastics. In fact, now I realise that this is an issue with Quiver too. Inside of an 
OutcomeRaise<Outcome<...>>
, doing 
raise(Absent)
will result in an output of 
Absent
instead of 
Failure(Absent)
s
Tiny fix.
Untitled.cpp
Untitled.cpp
All scenarios work for me.
Copy code
fun main() {
    val x = outcome<String, Int> {
        ProgressiveOutcome.Success(1).bind() + ProgressiveOutcome.Success(1).bind()
//        ProgressiveOutcome.Incomplete(41).bind()
//        "Failure".left().bind()
        ProgressiveOutcome.Failure("Failure").bind()
    }
    println(x)
}
This all works as expected
Also, 
raise("Failure")
I was indeed double wrapping 
Failure
, but 
Incomplete
works fine for me.
But now I think I know what you meant @Youssef Shoaib [MOD] but should be fixable
You meant 
raise(ProgressiveOutcome.Failure("Failure"))
rather than bind, right?
y
Yeah, that's what I intended. I'll write a reproducer quickly
c
that's easy to fix by simply adding a 
raise
overload
s
That won't even work because 
Raise
is fixed to 
E
, not 
ProgressiveOutcome
So it's a compile time error, unless you indeed explicitly add an overload.
But then I'd add 
failure(progress: Progress, e: E)
The neat thing is you can just run any 
Raise<E>.() -> A
based program inside your DSL
y
This typechecks but fails with a CCE:
Copy code
fun main() {
  fun consumeProgressiveOutcome(outcome: ProgressiveOutcome<String, Unit>) { }
  val x = outcome<ProgressiveOutcome<String, Unit>, Int> {
    raise(ProgressiveOutcome.Failure("error"))
  }
  when (x) {
    is ProgressiveOutcome.Failure -> {
      consumeProgressiveOutcome(x.failure)
    }
    else -> {}
  }
}
The way around this would be to use custom wrapper types inside 
OutcomeRaise
, and then unwrap inside 
outcome
, while insuring that those custom wrappers never escape to the outside world. The issue here is that we use 
Failure
and 
Incomplete
as signalling values internally, so sometimes we might mistaken legitimate values as signals
c
If the goal is to expose 
Raise<E>.() -> A
, isn't this simpler?
Copy code
@JvmInline
value class ProgressiveOutcomeDsl<Failure>(private val raise: Raise<ProgressiveOutcome.Unsuccessful<Failure>>) : Raise<Failure> {

	override fun raise(r: Failure): Nothing =
		raise(r, done())

	@JsName("raiseUnsuccessful")
	fun raise(failure: ProgressiveOutcome.Unsuccessful<Failure>): Nothing =
		raise.raise(failure)

	@JsName("raiseWithProgress")
	fun raise(failure: Failure, progress: Progress = done()): Nothing =
		raise(ProgressiveOutcome.Failure(failure, progress))

    // …bind and stuff
}
s
Here the original code is updated 😜 That looks good to me!
Much simpler yes
y
Yes that's a lot simpler because it doesn't use any signalling values, but you do get some unnecessary wrapping. It's probably acceptable given that you're raising a value with 2 properties anyways.
c
Unnecessary wrapping?
y
Everything is wrapped in Failure, but looking at it, it's actually not unnecessary. Never mind me, too little coffee :S. One possible criticism I'd have is it's hard to "compose" your DSL with the other 
Raise
functions. E.g. if someone calls 
recover
and inside calls 
raise(failure, progress)
, the raised value won't be caught by recover. If you're going all-in on contexts, then I'd suggest instead using 
context(Raise<ProgressiveOutcome.Unsuccessful<Failure>>, Raise<Failure>)
, and then exposing your 
raiseUnsuccessful
and 
raiseWithProgress
as extension funs. This way, you get support for 
recover
et al. You wouldn't even need your own 
value class
then
☕ 1
c
Everything is wrapped in Failure, but looking at it, it's actually not unnecessary.
Is it not? The return type of the DSL is 
ProgressiveOutcome<Failure, Value>
anyway, so the wrapping will happen by that point anyway, no?
If you're going all-in on contexts,
…I am, but I support multiple platforms, so although I can start to design with them in my mind, I can't use them 😕
E.g. if someone calls 
recover
and inside calls 
raise(failure, progress)
, the raised value won't be caught by recover.
I don't understand this situation, do you have an example?
y
Yes sorry double negative. I mean yes you're right the wrapping is necessary. Instaed of using contexts now, just make sure your API could be represented by extension funs and 
withError
calls in a contexts world
Copy code
outcome<String, Int> {
  recover({
    raise("String", Progress.blah(42))
  }) { 1 }
}
This should return 
Success(1)
ideally, but here I think it either results in a compiler error because of unspecified type params, or it results in 
Failure("String", Progress.blah(42))
. However, if `outcome`'s 
block
has 
context(Raise<Failure>, Raise<ProgressiveOutcome.Unsuccessful<Failure>>)
and your functions are defined as extensions on 
Raise<ProgressiveOutcome.Unsuccessful<Failure>>
, then everything works as expected and you get 1.
âž• 1
s
Yes, that is super bad... We fixed it with an overload in 
Ior
, etc
y
Even an overload is only a bandaid sadly. If one tries to use 
fold
or any other Raise builders, the issue reappears. The real solution (at least as far as I've prototyped) is to do away with the wrapper Raise classes and instead use contexts. For instance, 
IorRaise<E>
can be decomposed into 
context(Raise<E>, IorAccumulate<E>)
where 
IorAccumulate
deals with the accumulation logic, and 
Raise
does its thing. With such decomposition, everything works as expected because 
recover
simply replaces the 
Raise
part without touching the 
IorAccumulate
.
âž• 1
👀 1
c
Shouldn't 
@RaiseDsl
stop that from happening? It's a 
@DslMarker
, right?
s
It doesn't allow such fine-grained control
I mean, your DSL is still useable but there is this gotcha until receivers are stable.
c
That's a pretty big trap though.
s
With the recover bandaid that's what we currently have in Arrow for Option, Nullable, and Ior..
c
The second context receivers become binary-stable, I'm releasing a major version of all my libs lol
😂 1
y
@RaiseDsl
doesn't actually mark 
Raise
, and that's on purpose. 
either<String, _> { either<Int, _> { raise(42) } }
is expected to work. Hopefully once K2 is out fully, JB will give contexts more love.
âž• 1
c
Wait, really? I thought for sure this was forbidden.
s
It's important this kind-of stuff works, or 
flatMapLeft
is not possible to implement
👀 1
y
Also, even though 
@DslMarker
would forbid such usages implicitly, one can always use an explicit receiver. I'm also not sure whether 
context(Raise<A>, Raise<B>)
would be forbidden by 
@DslMarker
, but if it's forbidden, it'd be a huge nerf, and if it isn't forbidden, then we get to the same issue that we have here with 
recover
etc
c
Yes, but with an explicit receiver, this isn't ambiguous or surprising, no?
y
Oh yeah, you're right on that lol! I think the ability to raise to an outer receiver was deemed too important and hence we have to live with this gotcha for now. If 
DslMarker
had a way to only get mad based on type parameter value, then perhaps it would be applicable here, but alas.
a
I'm late to the party, but I wanted to add that we have some info in https://arrow-kt.io/learn/typed-errors/own-error-types/
y
Apparently those docs were created in response to Ivan. Funny how history repeats itself!
c
@Youssef Shoaib [MOD] ahah yes, it was back when I implemented the DSL for the regular 
Outcome
class, which doesn't have progress information
In the example in the documentation, we implement a 
Raise<Lce<Failure, Nothing>>
, which seems a bit strange to me. I won't be able to run raise programs over 
Raise<Failure>
.
y
The point is to encapsulate all the failure types that you have. If you read further ahead, it suggests instead to have a sub-hierarchy with all the failures, which is what you also cleverly suggested with 
ProgressiveOutcome.Unsuccessful
(Aw man I just realised how cool this could be if we had Union types and perhaps a way to remove an element from a Union, then we could do 
ProgressiveOutcome - ProgressiveOutcome.Successful
and perhaps make an easy DSL super-function that does exactly what those docs say to do)
c
I tried your tricky example, and I get a 
Not enough information to infer type variable Failure
on both the DSL and 
recover
😕
tbh, if we had union types, I would just done 
Incomplete | Failure<F>
🙂
y
recover<ProgressiveOutcome.Unsuccessful<...>, _>
should hopefully do the trick, then the outer one will need 
outcome<Nothing, _>
until your example gets more complicated
c
…it compiles, but indeed the 
raise
is for the outer scope. I don't like this at all.
y
2 options to solve this: • Contexts! • define a 
ProgressiveOutcomeDsl.recover
function
c
I don't have contexts… this a KMP lib
I wish 😅
😢 1
To be clear, I'm not complaining about the time it's taking… I understand how much work it is to get them to work properly and I certainly don't want them to be half-backed when we finally get them. …but damn do I want them
define a 
ProgressiveOutcomeDsl.recover
function
I'm going to end up the entirety of Arrow within that one class though
Well, for now I'm keeping all of this behind an opt-in, so it's fine.
y
Well, I guess people usually only really use 
recover
, hence why that's the bandaid we went for. One annoying option is to forego a custom DSL class entirely, and instead use Raise directly, and define all your methods as extensions on 
Raise<ProgressiveOutcome.Unsuccessful>
, even including 
bind
yes which is a shame
If you don't expect to be `bind`ing often then no DSL class is definitely the way to go. Then the improvement you'd get in the future with contexts is just a nicer 
bind
callsite
c
Well, in the future, I'm definitely all-in on context receivers so I essentially won't ever be `bind`ing
but at the moment I use extension functions quite a bit
y
That's the situation you have to weigh up then. With Arrow, bind is used often for 
nullable
, 
option
, etc hence why it was crucial to keep around (and well because this issue only surfaced later). If you can live with a 
bind(foo)
call, then that's the way to go. Btw, I know your library is multiplatform, but you can still provide nice syntax in the JVM module, so that could be a better trade-off
14 Views
Open in Slack
PreviousNext
Powered by