https://kotlinlang.org logo
Join Slack
Powered by
What is the current take on Kotlin's native Result...
# arrow
f
What is the current take on Kotlin's native Result vs eg Arrow Either/Try? I've used Either/Try in my previous team, about to join a team that prefer using Result instead. They seem kinda similar to me (Result needs throwables as errors which is a bit different but whatever) but then there are people that claim that Kotlin devs themselves advocate against using Result in your business code as it will catch all exceptions, which isn't really desirable (see https://stackoverflow.com/questions/70847513/when-and-how-to-use-result-in-kotlin)
s
You’ll notice that 
Result
only has one generic parameter, where 
Either
has two. That means that 
Either
can model everything that 
Result
can, but 
Result
cannot model everything that 
Either
can. If you need type information about your errors, 
Result
will not help you.
f
to be clear, I don't really agree with @elizarov when he says the following (because it flies in the face of functional error handling and moving trivial business errors to compile time, instead, explicitly advocating turning them into exceptions that blow up in runtime... imo, exceptions should be just that, exceptional, things like out of memory error etc that can't be recovered from, and other regular errors should be regular objects like any other)
Copy code
The first and foremost use of exceptions in Kotlin is to handle program logic errors. Exceptions are used to check preconditions and invariants in code that static type system cannot track down during compilation. For example, if you a have a function that updates order quantity that must be always positive by the business logic of the code, then you might write something like:

/** Updates order [quanity], must be positive. */
fun updateOrderQuanity(orderId: OrderId, quantity: Int) {
    require(quantity > 0) { "Quantity must be positive" }
    // proceed with update
}

Calling this function with a negative or zero quantity indicates a logic error in some other place in the code and throws an exception. The same happens when you try to get an element by an out of range index from a list, divide by zero, etc.
(https://elizarov.medium.com/kotlin-and-exceptions-8062f589d07)
@Sam so as far as Arrow is concerned, Either/Try is still endorsed and considered superior to runCatching and Result? Arrow isn't planning on deprecating Either or anything like that? (as was proposed for Option)
s
Yes, I don’t see 
Either
going anywhere. It has many use cases that aren’t addressed by the 
Result
type.
f
Agreed, I do think Either is superior to Result or just throwing exceptions, so I'm glad it's not going anywhere, lol That said, in case you weren't able to use Either, what's your take on Result as the next best thing? I think it's positive that it provides Either like functionality albeit not as flexible or powerful, but it's true that it's negative that it forces you to turn everything, even trivial business errors, into exceptions + it potentially catches far more than desirable... (but even then, intuitively I'd say that's still preferable over just throwing exceptions for everything... even if you end up catching an outofmemory error in your business code, at the resource layer you can just look for your business errors, return 4xx or whatever for them, and 5xx and log error for everything else)
s
Normally if I’m using `Throwable`s I throw them, and if I’m using 
Either
then my 
Left
type is typically some very descriptive sealed type that is not a 
Throwable
. The use case for 
Result
, where you want to use a 
Throwable
but not throw it, is in my experience a fairly small intersection. It comes up occasionally in framework code, where you want to shuttle an exception from one place to another. This is actually what it was designed for (specifically for the behind-the-scenes implementation of coroutines). But in my experience it isn’t typically relevant for day-to-day code.
f
What you're describing is exactly what I've been doing at previous gigs. I make sealed error classes for expectable business errors eg on a GET user id 555, at db layer we find out there is no such user (there's no way to guard against this with validation like Roman seems to think) - that's sealed error type UserNotFound so the resource layer looks something like
Copy code
@GET
fun getUser(id: Int) : User {
    return userService
        .getUser(id) //Either<GetUserError, User>
        .fold({200(it)}, {when UserNotFound 404 etc etc map other errors to correct status code})

}
this is my understanding of an example of idiomatic use of Either
p
^^^^ literally the code we have in production
libraries or modules return values in the domain
and only the outermost layer translates those to HTTP results
f
just like God intended 🙃 thanks everyone for clearing these subtleties up
s
imo, exceptions should be just that, exceptional
💯 this
c
Also, 
Result
is really not great. 
runCatching
catches 
CancellationException
and breaks coroutines, and as the others have said it's untyped.
f
@CLOVIS interesting, I didn't know the subtlety that runCatching breaks coroutines... this team I'm about to join are all in on coroutines so I'd love to understand how in case you have a minute to explain?
s
c
CancellationException
is used internally by coroutines. If you don't know exactly what you're doing, you should not touch it (not catch it, not rethrow it…). Having a 
catch(e: Exception)
or 
catch(e: Throwable)
in your code can create memory leaks or break coroutines in other ways. Arrow (at least, the newer versions) also use 
CancellationException
internally, and risk being broken too. 
runCatching
has a 
catch (e: Throwable)
, and should therefore not be used in coroutines. 
Either.catch
is clever and doesn't have this problem (safe to use with coroutines).
f
intuitively I'd guess cancellationexception might have to do with eg scheduling and the internals of the state machine of coroutines and yeah that's probably something you don't want to catch but leave to the language to figure out what to do with haha
c
The very simplified explanation is that it's thrown by a child coroutine when its parent kills it. If you catch it, you risk having the child continue to work even though it shouldn't, have the parent no know the child has died…
You never really have to touch it yourself though. Just remember that in coroutine code, 
catch (e: Throwable)
or 
catch (e: Exception)
has a very bad smell. But then again, it already has a bad smell in regular code. As long as you always catch specifically the type of exception you expect, or use clever functions like 
Either.catch
, you don't have to think about it.
@Sam thanks for the article, I'll play a bit more with it but indeed it sounds like a very good safety guard.
f
@CLOVIS> Arrow (at least, the newer versions) also use CancellationException internally, and risk being broken too. Have you reported this? Seems like a pretty important thing to report before it makes its way out into users projects
c
Oh, it's a feature! It makes Arrow much better 🙂
f
wat 😅
c
@pakoito and @simon.vergauwen, who already commented in this thread and are both Arrow maintainers, can explain it better than I can. CancellationException is not a bad thing, what's bad is code that catches everything (and catches it accidentally). It's just a lower implementation detail that we shouldn't concern ourselves with, because it's complicated to use and it's easy to mess it up.
s
Short version: the only (safe) ways to exit from a code block are to throw an exception or to return. Arrow provides 
effect
scopes that allow you to exit early without a return, hence exceptions are required for correct control flow. Any code that catches broad categories of throwables risks catching these control flow exceptions; this is an unavoidable trade-off of the functionality.
c
Deep down, it's always the same thing: don't catch broad categories of exceptions. It was already important in the Java days, and it's still important with Coroutines. But that's how Result works, so don't use it. (for the reason: Result is also more or less a technical detail of coroutines…)
I created an issue to add this to 
runCatching
's documentation, but they haven't replied yet.
f
@CLOVIS I'm a little confused, maybe I'm misunderstanding you... are you saying the fact Arrow doesn't catch these CancellationExceptions makes Arrow safe? Or are you saying that Arrow does use it internally and risks being broken? (because if Arrow risks being broken, I don't understand why you say it's a feature that makes Arrow better - Arrow being broken doesn't seem like a feature that makes it better 😅 )
+ In any case, what does all this mean in practice? I promise not to use 
runCatching
or 
try catch (all exceptions)
, is that enough to make this a non issue or..?
c
If you yourself catch 
CancellationException
without knowing exactly what you're doing, you can break Coroutines and Arrow. Coroutines and Arrow themselves are written by people who know what they are doing, and are very careful to use it correctly.
f
@CLOVIS ok, I was indeed misunderstanding you, thanks for clearing that up, I see what you were actually saying now
p
c
If you never catch 
CancellationException
, you have nothing to worry about, yes.
p
Arrow retrows Nonfatal exceptions
nonFatalOrThrow
is peppered through code that interacts with coroutines
I haven’t been involved in code since before the rewrite to use kx.coroutines, but knowing Simon it’s all nicely done and cleanly tested :D
c
IMO 
NonFatal
should have been a part of Coroutines itself. It's great that Arrow makes it so easy though 🙂
f
right... but Arrow users themselves should rarely if ever find themselves writing their own code that calls nonFatal, nonFatalOrThrow etc, right? We should just use Arrow and let Arrow handle itself
c
If you don't write code that catches broad exception categories, you never need to use it, yes. If you do, that's the clever bit you need to use to avoid breaking everything (and Arrow needs to do it internally).
But again, there's no need to overthink this too much. Catching broad categories of exception has always been an anti-pattern, and this is only one of the reasons you shouldn't do it. If you don't do it, there's nothing else to worry about.
f
Got it, thanks. Side note, but it borderline brings me to tears how friggin amazing this community is. I remember the days of random IRC when you would jump into literally any channel and ask literally any question and the only replies you would get would be bullying, trolling, and every other type of toxic unhelpful bs. Likewise StackOverflow these days seem hell bent on closing every question because someone answered a similar but not the same question 20 years ago. Big, big thanks to everyone who contributes here, it's nothing short of amazing, can't thank you all enough.
s
Comments are not for extended discussion. This conversation has been moved to chat. 🧌stackoverflow
c
Btw I've been reading the article mentioned above, and it really is great. I'll save it somewhere.
Just noticed you're the author 😅
f
sorry to revive this old thread but I was just curious if y'all have anything to say about this re writing code using runcatching and result
inside a coroutine, yeah you need to be careful if you are using the cancellation mechanism, but we're not really doing that. we're not generally launching our own coroutines. ktor provides us the coroutine per request, so we can easily operate within that. where we use run catching is to trap a database error for example.
I'm simply not familiar enough with it all to know if the above is fine or whether it could potentially result in the issues discussed in the thread (even then, arguably, the potential for issues is pretty small, no? especially if the machine isn't struggling, then there would be little reason for the parents/scheduler to kill children coroutines etc)
c
runCatching
, when used incorrectly (and if you're using it for error management, you are using it incorrectly) breaks coroutines and every other exception-based library. It doesn't matter if it's inside Ktor or anywhere else.
If you want to catch Database errors, use Arrow's 
Either.catch
, a specific 
try…catch
with a specific exception type (do NOT catch Throwable, Exception or RuntimeException!), or just let it bubble up normally and use Ktor's StatusPages plugin to convert it into a proper HTTP error code.
f
ok 😅 this is in the context of a new gig where runcatching is used in the fashion described I did raise it pointing out what has been said in this thread but the team seems unconcerned so not much else I can do (unlike my previous gig, arrow isn't used here, maybe in the future 🤷) really appreciate everyone's input and really sorry if I'm being repetetive asking about it again 🙂
c
Good luck with that… but if you are ever stuck trying to debug why a function is not called, or why a coroutine never terminates, or why it continues executing even through it's been cancelled: it's because of 
runCatching
.
Good luck trying to find which one though. It's close to impossible to debug
f
gonna try to explain with a concrete example why runCatching used as described is suboptimal, what do you think? is what i'm saying accurate? sorry for harping on about this but without a simple, concrete example i don't think i can get any traction moving away from it context: runCatching is being used in prod code, mostly as a shortcut to get a Kotlin Result where Result is preferred over Arrow Either because Result has similar functionality and is native to Kotlin imagine the following Ktor endpoint (which, for a given call to it, becomes our parent suspend fun)
Copy code
kotlin
fun Route.myEndpoints(
    myClient: MyClient,
    myDAO: MyDAO
) {
    get("/my-endpoint") {
        myClient
            .makeACall()
            .flatMap { foo ->
                myDAO
                    .query(
                        "select * from foo where foo.id = :", 
                        foo.id, 
                        FooMapper
                    )
            }.fold (
                { call.respond(HttpStatusCode.OK, it) },
                { call.respond(HttpStatusCode.InternalServerError) },
            )
    }
}
Copy code
kotlin
class MyClient(private val client: HttpClient) {
    suspend fun makeACall(): Result<Foo> = runCatching {
        // imagine this takes 5 seconds
        client
            .post("some/other/endpoint") {
                setBody("""{"foo":"bar"}""")
                contentType(ContentType.Application.Json)
                accept(ContentType.Application.Json)
            }.body() 
    }
}
Copy code
kotlin
class MyDAO(private val ds: DataSource) {
    suspend fun <T> query(
        // arguments
    ): Either<DbError, T> = runCatching {
            // imagine this takes 5 seconds
            template.queryForObject(sql, args, mapper) 
        }
    }
}
given a client calling my-endpoint (and this is but one of many calls to many endpoints in the app) because runCatching catches Throwable, which is the superclass that contains both JVM Errors (which should never be caught by anything) and Exception a) if the machine that our app is running on experienced some catastrophic JVM Error that is not recoverable or controllable by us, eg OutOfMemoryError, InternalError or StackOverflowError, it would be caught by the Result and we would happily go on pretending that that never happened (even though what should have happened is probably the instance should have died so it can be replaced by another by k8s or whatever) b) imagine the client has a timeout of 2 seconds against our endpoint ie after 2 seconds they terminate the call and no longer wait for a response note that our endpoint performs two operations sequentially, each taking 5 seconds for a total of 10 seconds before responding we could perform them simultaneously using awaitAll and cut our response time down to 5 seconds, but it wouldn't matter - the client only waits for 2 seconds anyway before closing the connection our Ktor endpoint, effectively the parent suspend fun, will detect that the client has closed the connection and is no longer listening, and send a cancel/interrupt to its child coroutine that it started with runCatching but because runCatching catches Throwable ie including CancellationException, the child coroutine that it started with runCatching will keep running not only until it gets a response, it will potentially keep the parent and itself alive as zombie coroutines indefinitely (because the cancellation was generated but caught so now they're just hanging around) eventually, given enough calls like this to my-endpoint, this will result in memory leaks and other unpredictable, hard to debug behavior for the instance running the app
+ just as a side note, i do think some people find the whole "never catch throwable" a bit confusing when Either.catch out of the box has a throwable on the left (i know arrow doesn't ACTUALLY catch throwable, you carefully only catch those errors you should, and rethrow the rest, but still, it looks like basically anything that goes wrong including outofmemory Error class errors could be there on the left)
c
Yes, that's a good summary. 
Result
really isn't made for error management. IMO it shouldn't have been added to the standard library because people expect this to be correct, but it's very wrong.
🙌 1
f
100% agree, it's one in a growing list of things Kotlin is getting wrong 😬 don't get me wrong I love Kotlin but really wish they wouldn't add so many footguns to the language and libraries 😅
c
Well, compared to C++ or JS we're still fine 🙂 In general though, exceptions are almost always footguns. I haven't really followed the debate when Result was introduced, but if I recall it was originally an implementation details of coroutines and it was later added to the standard library because of pressure from devs who wanted to use it as a first-party Either type. It's really not one, though…
1438 Views
Open in Slack
PreviousNext
Powered by