Dear people who always wondered "Should I stop thr...

# feed

[Joost Klitsie]

Dear people who always wondered "Should I stop throwing exceptions?", here is something to read if you are bored:
https://medium.com/@joostklitsie/stop-throwing-exceptions-4282c8472027
Please let me know what you think 🙂

[CLOVIS]

>

suspend fun run(): Result<Something> = fetchLoggedInUserUseCase.run()
>         .flatMap { user -> fetchSomeSecretUseCase.run(user) }
>         .flatMap { secret -> someRepository.fetchSomethingWithSecret(secret) }

> This, in my humble opinion, is extremely readable.
It is readable, but I wouldn't say 'extremely'. There's a lot of boilerplate here. But thankfully, in Kotlin, we can do much better, for example, with #arrow :

suspend fun run() = either {
    val user = fetchLoggedInUserUseCase().run()
    val secret = fetchSomeSecretUseCase().run(user)
    someRepository.fetchSomethingWithSecret(secret)
}

This is the same code, with the same typed error handling. This one, I would agree is extremely readable 🙂

[CLOVIS]

fun recoverExample(initialResult: Result<String>): Result<String>
    = initialResult.recover { "Will be returned if initialResult is failure" }

If you're using coroutines, and you seem to be, there is a bug in this code. It should be

suspend fun recoverExample(initialResult: Result<String>): Result<String> =
    initialResult.recover {
        currentCoroutineContext().ensureActive()
        "Will be returned if initialResult is failure"
    }

See https://betterprogramming.pub/the-silent-killer-thats-crashing-your-coroutines-9171d1e8f79b

[CLOVIS]

Same in your flatRecover

[Joost Klitsie]

@CLOVIS thanks for taking the time 🙂 your first example: that would be similar runCatching {} that returns result objects. The problem there is that is hard to tell when a function throws an error. In that case, I wouldn't even bother putting that into a use case, but wrap it in a runCatching further down (like from where we call a use case and map a result). However, still problem remains that we will probably forget to catch stuff

[CLOVIS]

``` suspend fun fetchSomethingWithSecret(

secret: String,

): Result<Something> = memoryCache.get(secret).flatRecover {

database.get(secret).flatRecover {

network.get(secret).onSuccess { something ->

database.set(secret, something)

}

}.onSuccess { stuff ->

memoryCache.set(secret, something)

}

}```

OMG. It is like functional programming and awesomeness had a baby.

Same code, with Arrow:

suspend fun fetchSomethingWithSecret(
    secret: String,
) = recover(
    block = {
        memoryCache.get(secret)
    },
    recover = {
        val response = network.get(secret)
        database.set(secret, response)
        memoryCache.set(secret, response)
        response
    }
)

[Joost Klitsie]

The recover (or flatRecover) one you mentioned doesn't catch anything. It simply maps a result to something else in case it is in a failure state. Therefore we do not need to ensure any activeness because any code up the stream that would throw things should throw cancellations/ensureactive in the catching part

[Joost Klitsie]

But you are right, in my example I mention runCatching, which does not do this on its own and it can be improved there

[CLOVIS]

The problem there is that is hard to tell when a function throws an error.

I kept the code exactly the same as yours. Both versions return a result object.

[Joost Klitsie]

but the biggest points is: if you don't try -> throw -> catch things, then you don't have to worry about it

[CLOVIS]

> The recover (or flatRecover) one you mentioned doesn't catch anything. It simply maps a result to something else in case it is in a failure state.
The problem is not catching, it's executing code after a CE has been thrown. If a CE has been caught into the result, your recover and flatRecover will execute the recovering code, which is a leak. Code should not run after cancellation.

[Joost Klitsie]

well then you shouldn't catch it to begin with 😉

[CLOVIS]

That means forbidding runCatching , then. How do you create your result objects?

[Joost Klitsie]

example which I like:
fun fetchDataFromMemory(): Result<String> = memoryCache.get()?.let(::success) ?: failure(NotFoundException("stuff not found"))

example which I do not like:
fun fetchDataFromMemory(): String = memoryCache.get() ?: throw NotFoundException("Stuff not found)

[Joost Klitsie]

I agree with you that runCatching in that case should probably not be used and a cancellation safe replacement should be used instead

[CLOVIS]

Why do you prefer the first one?

[Joost Klitsie]

Because try/catchinig is not a good solution for a programming language that does not have checked exceptions
Also, I am an Android engineer so forgetting to catch things will ruin my customer's day as their app will crash 😉

[CLOVIS]

That, I agree with. However, the examples I mentioned using Arrow don't use exceptions either 🙂

[Joost Klitsie]

I am unfamiliar with arrow, so looking it up as we speak 😉

[CLOVIS]

I recommend

https://www.youtube.com/watch?v=JcFEI8_af3g▾

, which is about exactly this

[swanti]

What is the point of using so many `flatmap`s in this snippet?

suspend fun run(): Result<Something> = fetchLoggedInUserUseCase.run()
        .flatMap { user -> fetchSomeSecretUseCase.run(user) }
        .flatMap { secret -> someRepository.fetchSomethingWithSecret(secret) }

Wouldn't it be nicer to do something like:

suspend fun run(): Result<Something> = runCatching { 
val user = fetchLoggedInUserUseCase.run().getOrThrow()
val secret = fetchSomeSecretUseCase.run(user).getOrThrow()
someRepository.fetchSomethingWithSecret(secret).getOrThrow()
}

I think getOrThrow is nicer because it doesn't add indentation and it's very clear what it does.

[CLOVIS]

@swanti you are catching CancellationException, which risks hiding the cause of errors. See https://betterprogramming.pub/the-silent-killer-thats-crashing-your-coroutines-9171d1e8f79b. Also, your version is much slower: creating exceptions is not free.

The version in the article doesn't have these problems (neither does the Arrow version).

[Joost Klitsie]

@CLOVIS I think in your Arrow example you miss the .bind() in there

[CLOVIS]

.bind is only needed to convert between Either and the Raise DSL. If your entire call stack uses the Raise DSL, you never need to convert, and thus you never need to call .bind

[Joost Klitsie]

@swanti I think it is more elegant to not throw/catch data like that, and of course the cancellation error there.
I think with a flatMap you have an "almost" out of the box solution for a more functional way of programming

[Joost Klitsie]

@CLOVIS if I use your example in my IDE it is not working

[CLOVIS]

In your example, you are creating a Result functions by combining multiple Result functions. In my example, I am creating a Raise function by combining multiple Raise functions.

[CLOVIS]

I could write a full example for this if you want, but not now

[Joost Klitsie]

this is how one of the interfaces looks like:

interface FetchSomeSecretUseCase {
    suspend fun run(user: User): Either<Exception, String>
}

[Joost Klitsie]

are you saying this should not return an Either but something else?

[CLOVIS]

If you use

suspend fun run(user: User): Either<Exception, String>

then yes, you have to use .bind()

However, you can write

suspend fun Raise<Exception>.run(user: User): String

or

context(Raise<Exception>)
suspend fun run(user: User): String

and then you don't have to use .bind()

[CLOVIS]

But you're right, my example is not perfect. I should have written

suspend fun Raise<Exception>.run(): Something {
    val user = fetchLoggedInUserUseCase().run()
    val secret = fetchSomeSecretUseCase().run(user)
    return someRepository.fetchSomethingWithSecret(secret)
}

so it's clear they all use Raise

[swanti]

@CLOVIS I hadn't noticed the function was a suspend function, but the point still stands, you can create a function called runSuspendCatching. Regarding creating exceptions, you only create one, which doesn't seem like a huge performance hit.

[CLOVIS]

I think your version is ok if you manage CEs correctly. I don't think there is anything bad about your examples, it's just that I think Arrow has the same benefits with a lot less code 👍

[CLOVIS]

To rephrase, if Arrow didn't exist, I would agree with you that your way to do things is the best way

[swanti]

I have never felt the need to checkout arrow, but it seems that having to define the function as a Raise<Exception> extension function would limit what you can do. You can't have a List<T>.Raise<Exception>.run() function or whatever. But maybe there is a different way to handle cases like those.

[CLOVIS]

Yes, this is a current limitation of Kotlin. The aim is to use with context receivers, so you can define

context(Raise<Exception>)
fun List<T>.run() { … }

If you're on the JVM or Android, you can enable the context receiver prototype, but this isn't yet available for all platforms.

[Joost Klitsie]

@CLOVIS I see I would have to add the context(Raise<Exception>) to all the functions in that case. So I guess that is the tradeoff there, you cannot magically get out of it without specifying at least the parts that may raise an error (or go back to throwing errors and you don't have to define anything 😄)
So in that case, my Result with flatMap approach would be more vanilla, but I agree that arrow does look cool as well and I will check it in more detail when I will have time soon

[Joost Klitsie]

anyway, both arrow or result approaches don't throw anything anymore 🙂

[Joost Klitsie]

which is at the core of my saltiness 😛

[CLOVIS]

No, it's the same thing. Your functions have to return Result to be part of your pattern. Arrow's functions have to have a receiver on Raise to be part of the pattern, it's the same.

Of course, you have to create a conversion function to enter either world. In the Result world, you would use something like

runCatching {
    yourCode…
}.recover { coroutineContext.ensureActive() }

and now you're safe (you're still catching OutOfMemoryError, StackOverflowError, etc though).

In the Arrow world, the function is just called catch :

catch {
    yourCode…
}

It automatically handles CancellationException and all other nasty things you don't want to catch

[gildor]

Unpopular opinion:
The same code with try/catch on top level (or where you need recover) is easier to write and especially read, also potentially more flexible in case if you need more logic later

[CLOVIS]

Well, the problems are "what to catch" and the performance of exceptions. I don't think your opinion would be unpopular if we could avoid these two problems

[gildor]

runCatching are not better though in terms of performance (slightly worse even)
Also I would argue performance of them is not always the problem, depends on the code of course

I understand issue with exceptions, I just feel that Result, Either are not solutions. Yes, they solve issue to make it explicit, but they do not help with "what to catch" and pollute code a lot
I really tried hard and experiment a lot in our project, and I should admit, failed to convince myself that it actually better

[CLOVIS]

runCatching are not better though in terms of performance (slightly worse even)

Compared to what? runCatching is an inline function, there's no difference between using it or writing a try…catch yourself. The expensive things are the exception constructors and the throw mechanism.

they do not help with "what to catch"

I disagree. Using Result (assuming you properly catch what needs to be caught, meaning you don't use runCatching) and Either allows separation of concerns: the code that may crash is responsible for catching the exception or representing any other error, and communicates that to the part of the codebase that must deal with it. That second part doesn't need to know anything about the error conditions, it doesn't even need to know which exceptions are possible, because they are typesafely described by the wrapper.

pollute code a lot

Well, we're working on it 😕

[gildor]

Yes, I see what you mean now about "what to catch"

I mean it solves issue of "when to catch", but not "what to catch"

Compared to what? runCatching is an inline function

It creates exception anyway, with stacktrace. Also creates one more, non value, class Failure to wrap it. Not a big deal, but still

[gildor]

we're working on it

Yeah, let's see how Union types for exception will affect it. But I don't think Result/Either will ever be good without language support

[CLOVIS]

Comparing idiomatic Kotlin:

fun logIn(username: String, password: String, passwordConfirmation: String): User {
    require(username.isNotBlank()) { "Username shouldn't be empty: '$username'" }
    require(password.length in 8..64) { "Password is not the correct size" }
    require(password == passwordConfirmation) { "Passwords don't match" }

    …  
}

with the current state of Arrow with context parameters:

sealed class LogInError {
    data object UsernameShouldntBeBlank : LogInError()
    data object PasswordIncorrectSize : LogInError()
    data object PasswordsDoNotMatch : LogInError()
}

context(_: Raise<UserError>)
fun logIn(username: String, password: String, passwordConfirmation: String): User {
    ensure(username.isNotBlank()) { UsernameShouldntBeBlank }
    ensure(password.length in 8..64) { PasswordIncorrectSize }
    ensure(password == passwordConfirmation) { PasswordsDoNotMatch }

    …
}

I don't think it's possible to do much better than that for code pollution… The only change to the function itself is adding the context declaration to declare the error type, which seems reasonable to me.

[gildor]

The expensive things are the exception constructors and the throw mechanism.

Yes, but you do not get rid of them if replace with Result

And don't forget, Exceptions have stacktrace, yes we pay for them, but also they provide a lot of debug information, not available for algebraic types

[gildor]

This looks neat, I agree with you, may look into it. But examples from the article, not really (same as our own attempts to use Result)

[CLOVIS]

Btw, the equivalent with the union type prototype will be something like

error sealed class LogInError {
    error data object UsernameShouldntBeBlank : LogInError()
    error data object PasswordIncorrectSize : LogInError()
    error data object PasswordsDoNotMatch : LogInError()
}

fun logIn(username: String, password: String, passwordConfirmation: String): User | LogInError {
    if (username.isBlank()) return UsernameShouldntBeBlank
    if (password.length !in 8..64) return PasswordIncorrectSize
    if (password != passwordConfirmation) return PasswordsDoNotMatch

    …
}

but the worst part is the chaining syntax.

Current Kotlin:

fun foo(i: Int): Bar =
    bar(baz(i))

Current Arrow with context parameters:

context(_: Raise<FooError>)
fun foo(i: Int): Bar =
    bar(baz(i))

Current Arrow with current Kotlin:

fun Raise<FooError>.foo(i: Int): Bar =
    bar(baz(i))

Union types prototype:

fun foo(i: Int): Bar | FooError =
    baz(i)
        !.let { bar(it) }

So far, being a language feature doesn't imply less code pollution 😅

[CLOVIS]

same as our own attempts to use Result

That, I completely agree with. Result was never designed for error handling, and it's really inconvenient to use as such.

[gildor]

Either or other solutions are not better... either 😅

[gildor]

So far, being a language feature doesn't imply less code pollution

Yes, for sure, I just don't really understand all implications yet, so hard to comment on it. Why it may be promising, is improvement for marking core exceptions as error object

[gildor]

I would actually fine with this, without let, at least I don't have to use orElse/recover and other stuff which breaks natural language constructs like if/else/when

fun foo(i: Int): Bar | FooError {
    val res = baz(i)!!
    return bar(res)
}

[CLOVIS]

As far as I know, this syntax is not part of the proposal at the moment.

It's interesting, because this proposal looks a lot like Arrow did ~3 years ago, which was changed because it polluted the code too much and was too confusing to users:

fun foo(i: Int): Either<FooError, Bar> = either {
    val res = !baz(i)
    bar(res)
}

[gildor]

yes, it's not in the proposal, I know, just that something like this would be not so bad

[gildor]

Judging based on experience with nullability

[Joost Klitsie]

There was an interesting talk about this:

https://www.youtube.com/watch?v=tAGJ5zJXJ7w▾

[gildor]

Potentially it will be !! same as with null, not ! but I saw discussion in the issue and potential foo!!!!

[Joost Klitsie]

around 27 minutes

[Joost Klitsie]

union types with errors

[gildor]

yes, this what we exactly discussing

[CLOVIS]

This is the talk about union types:

https://www.youtube.com/watch?v=3uNpmhHwkuQ▾

[gildor]

This issue also has more details and interesting discussion too
https://youtrack.jetbrains.com/issue/KT-68296/Union-Types-for-Errors

[Joost Klitsie]

I like the concept:

fun getSomething(): Something | SomeError
 = getUser()!.let { user -> 
     getSecret(user) 
   }!.let { secret -> 
     getSomethingWithSecret(secret) 
   }

[gildor]

I really don't like it at all

[gildor]

I would never allow such code

[CLOVIS]

Compare that to the current code:

Kotlin (untyped errors):

fun getSomething(): Something = 
    getSomethingWithSecret(getSecret(getUser()))

Arrow (exact same benefits):

context(_: Raise<SomeError>)
fun getSomething(): Something =
    getSomethingWithSecret(getSecret(getUser()))

Currently, this proposal is just a prototype. But if it gets implemented like this, I'm definitely not using it

[gildor]

But RaiseSomeError doesn't allow to show different types of errors though, until it has some kind of OtherError(t: Throwable) in hierarchy

[gildor]

I don't want to use chains syntax, I just want sequential, not nested one, which potentially can give me info about possible exceptions from method

[CLOVIS]

But Raise<SomeError> doesn't allow to show different types of errors though

You can context(_: Raise<SomeError>, _: Raise<SomeOtherError>)
to emulate union types

[gildor]

I see, pretty cool