Rob Elliot

    Rob Elliot

    1 year ago
    I’m surprised by the definition of
    fun <T, R> T.runCatching(block: T.() -> R): Result<R>
    - it catches
    Throwable
    , and hence
    Error
    , including things you almost certainly don’t want to catch like
    OutOfMemoryError
    . It also makes no attempt to propagate
    InterruptedException
    (or reset the interrupted status of the Thread when catching it). Am I missing something? Or just woefully out of date in my JVM exception handling?
    Oliver.O

    Oliver.O

    1 year ago
    Hmm, it returns a
    Result
    , expecting its caller to examine that for failures and respond accordingly. What else were you expecting?
    christophsturm

    christophsturm

    1 year ago
    I think you are totally right and I don’t use runCatching for exactly that reason. (except in unit tests)
    its totally ok to catch throwable on the top level and (try to) log it, as long you rethrow it, but the “handling” part of that tweet is misleading IMO
    Rob Elliot

    Rob Elliot

    1 year ago
    My understanding is that
    Error
    is intended to represent all the things you should, as a general rule, never catch because they represent things you can’t sensibly handle. As a general rule they should hit the top, and halt the program or thread. (Of course there are cases where you want to catch them, but they are very much the exception (see what I did there?) rather than the rule.) In addition, from my reading of JCIP catching
    InterruptedException
    accidentally by virtue of catching
    Throwable
    or
    Exception
    is dangerous because you should reset the interrupted status of the thread when you do so, otherwise interruption based cancel semantics don’t work. So it’s generally better to propagate
    InterruptedException
    - unless you were catching it deliberately you probably don’t want to accidentally undo interruption. (On which basis
    InterruptedException
    should really be a third direct subtype of
    Throwable
    rather than a subtype of
    Exception
    , but that ship sailed 25+ years ago.) My assumption was that 99.999% of the time someone using
    runCatching
    is not checking the result for Errors and rethrowing them, nor for InterruptedException & handling it appropriately, but is looking to bridge “control flow” exception throwing code into return value style code. In which case if I’m right about the above they shouldn’t actually want to use
    runCatching
    .
    christophsturm

    christophsturm

    1 year ago
    since try/catch is an expression in kotlin i see no advantages of using runCatching
    Rob Elliot

    Rob Elliot

    1 year ago
    You can’t optimise
    tailrec
    inside a
    try/catch
    block, which was my immediate reason for turning it into a
    Result
    & using
    when
    on it.
    Oliver.O

    Oliver.O

    1 year ago
    OK, its certainly a bad idea to use
    runCatching
    without exhaustively handling its failures and re-throwing everything that can't be handled. I'd say, having a form of
    runCatching
    which tries to single out specific failures by throwing and passes others would just be a pseudo-solution: Important failure modes might still be silently ignored by the caller. When there's no discipline and a very good use case, it's better not to consider using it at all.
    raulraja

    raulraja

    1 year ago
    The scala standard lib has the notion of matching on non fatal Throwables https://dotty.epfl.ch/api/scala/util/control/NonFatal$.html. Similar in Arrow we have https://github.com/arrow-kt/arrow/blob/0afdd9a7cc/arrow-libs/core/arrow-core/src/jvmMain/kotlin/arrow/core/NonFatal.kt which we generally use when we want to rethrow all we can’t really recover from. Perhaps there is a place for something similar in the std lib.
    christophsturm

    christophsturm

    1 year ago
    thats great. maybe runCatching should use a list like that
    ilya.gorbunov

    ilya.gorbunov

    1 year ago
    We believe that the decision whether a caught exception should be rethrown or proceed with the result is highly application specific, so we won't be engraving it in the functions working with Result. However you can create your own extension for Result, say
    Result<T>.rethrowFatal()
    , analyze there if an exception is fatal according to your criteria and rethrow it immediately.