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.

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

Error handling in the Kotlin language #81

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@BenLeggiero
Copy link

commented Aug 16, 2017

Full text is here

Synopsis

This proposal attempts to give Kotlin's error handling the same delight, peace-of-mind, and unambiguous terseness that the language already generally has.

Quick example:

fun danger(input: Int) throws : String {
    val output = input % 2
    if (0 == output) {
        throw MyCustomException()
    }
    else {
        return output.toString()
    }
}

fun main(args: Array<String>) {
    val optionalString = try? danger(1)

    do {
        val justString = try danger(2)
    } catch (x: MyCustomException) {
        println("Caught my custom exception: $x")
    } catch {
        println("Caught some unexpected exception! I don't care what it was.")
    }

    val crashesIfAnythingIsThrown = try!! danger(4)
}
@elizarov

This comment has been minimized.

Copy link
Member

commented Aug 16, 2017

You should address the problem of backwards compatibility with this proposal. This is a major breaking change to the language and thus it is a no-go in Kotlin's world (we have no plans to turn Kotlin into Swift in this respect). I don't think there can be a discussion on the merits of this proposal until it addresses the issue of introducing those changes without breaking existing code.

@BenLeggiero

This comment has been minimized.

Copy link
Author

commented Aug 16, 2017

@elizarov I understand and took quite some time to consider such a breaking change. Here are my reasons:

  1. I don't intend for this to be a 1.x change, but a x.0 change. Essentially, in my opinion, error handling needs so much rework that it must break some existing code.

  2. When a developer upgrades to a new version of a language, they acknowledge that their existing code probably won't compile/run the same for some reason or another.

  3. The change should be easily aided by a migration tool, in this way maintaining current behavior:

    1. Replace all try { ... } catch patterns with do { ... } catch
    2. [not so sure about this one] Put throws in the definition of closures that are the sole argument of an inline fun
    3. Put rethrows in the signature of any function that only takes throwing a closure
    4. Place throws in the signature of any function that is declared with @Throws
    5. Place try before any call to a throwing function
    6. Place throws in the signature of any remaining function containing a try outside a do { ... } catch pattern
    7. If the migration tool ran into any ambiguities, present the user with a list of them to resolve manually

Of course, migration tool designers might have better ideas, but this is a start.

@yole

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2017

I think (1) is a key point where we disagree. Having successfully used Kotlin in many projects at JetBrains and listened to feedback from the users of Kotlin pre-1.0 and in the 1.0.x and 1.1.x timeframe, we do not see that error handling in Kotlin isn't working as designed. Choosing an error handling strategy is a major design choice in a language, and we made our choice consciously, knowing which alternative approaches exist. We do not see any evidence that the approach we chose is broken.

@artem-zinnatullin

This comment has been minimized.

Copy link

commented Aug 16, 2017

Moreover the more you use Kotlin the less you use exceptions as a way to deal with error states, but more as a way to actually crash your program if something gone terribly wrong with enough info to trace it back. Maybe it's just my experience though.

Kotlin sealed classes feels like a much more appropriate way to introduce success, empty, error, etc states for the data types in the code.

@kingsleyadio

This comment has been minimized.

Copy link

commented Aug 16, 2017

I'm pasting my previous comment on the proposal here for visibility:

IMO, making code that throws an exception fail compile time checking might be a little too strict, and we might end up entirely defeating Kotlin's no-checked exceptions. I'd rather go with your Alternative 1 (re-using existing @throw and @Throws). And exposing an opt-in flag that when enabled, shows a compile time WARNING at call sites where the @Throws have not been properly handled.
On the IDE, I'd suggest highlighting these call sites and providing quick fixes to handle accordingly

Call sites: expressions that call methods with:
checked exceptions (Java interop) or
those that are annotated with @Throws (Kotlin)

I like your use cases (call sites) B, C, D and E.
B and C are features we already have today using try/catch
Regarding D and E. Again, this could be a form of opt in feature, such that current code still continues to work, and once "the flag" is enabled, current code throws "Warnings" until appropriate fixes (any of B, C, D or E) are applied.

Thinking about this, it might make sense to add a parameter (e.g lenient: Boolean) to @Throws to imply whether or not there should be a warning at the call site.
You could see it as a regular case of checked/unchecked exceptions, but with a more flexible design. Checked ones only show a warning/highlight (instead of a strict compile time failure)

@BenLeggiero

This comment has been minimized.

Copy link
Author

commented Aug 19, 2017

Hey all! Sorry it took me a couple days to respond. Here's a batch:


@yole

  1. And I expected that would be the main point of contention in this proposal. Unfortunately, I didn't have the luxury of knowing Kotlin even existed before 1.0, so I wasn't able to take part in the initial feedback phase. This is me saying what I would've said then, anyway. I hope we can at least agree that the whole reason KEEP allows pull requests and community proposals and feedback is because Kotlin is not a perfect language, and some of those imperfections will either stay forever, or require a breaking change to fix. Also, I'm 100% open to proposals for additional alternatives; I'm sure I didn't think of all of them.

The reason I say that Kotlin's error handling is "broken" is because you can have this:

class Crash: Exception("Crash!")

fun danger(i: Int): Int {
    val ret = i % 2
    return when (ret) {
        0 -> throw Crash()
        else -> ret
    }
}

fun main(args: Array<String>) {
    println(danger(1))
    println(danger(2))
}

Here, we only know by examining the danger function that it ever even throws, so when it's actually called, Kotlin never even warns us that we might need to catch an exception. In this example, we find out rather quickly, but in the real world (especially using an external SDK), a function's reason for crashing might be very rare, and it might never crash for the developer, but the users always have a less-favorable environment so it might crash for them, possibly in a way that's hard to track down.

In my opinion, this is not something that should be a problem in modern languages.


@artem-zinnatullin

I tried to make it clear in my proposal, but the main driving reason I did this is because I import JARs made with Java/Scala/etc. and use the libraries of Java SE, Android, etc. which are very eager to throw exceptions, especially on I/O and networking calls. Maybe one day, we could use Kotlin for these, but as it stands now, Kotlin stdlib is so barebones you need the help of external SDKs which love throwing, so Kotlin needs to be good at catching (notice most of my proposed syntax involves catching, and not so much for throwing). Right now, Kotlin is about as good as Java in this regard.


@kingsleyadio

Call Sites B, C, D, and E were the real meat of this anyway. I threw A in there because it seemed reasonable given how often people ignore the actual value of the thrown exception, and how unintuitive it is to catch Throwable. I'm not tied to Call Site A.

I love your idea to have syntax that makes the exceptions lenient. This reminds me of Swift's @discardableResult, which silences the default warning of not using a return value. Of course, to fit in with this proposal, it'd be a keyword. If we went with your favorite alternative, it obviously wouldn't.

@yole

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2017

@BenLeggiero Do you actually think that we were unaware of the possibility to write such a code snippet when we were designing Kotlin's error handling?

To expand on this, the only reason to make a major, backwards incompatible change to the design of a core part of the language is new information. Like, for example, a new breakthrough in solving a problem explored in another language or a research paper. Or the discovery that some of our assumptions do not hold in a specific important use case that we weren't aware of. Or unanimous feedback from a large part of the community that a shortcoming in the design of the language prevents them from adopting it.

In this case, I see no new information. We've always known that Java libraries like to throw exceptions. We've also known how inconsistent the design is (you need to check exceptions from closing a socket, but don't need to check exceptions when you parse a number to a string). We know how much code in enterprise apps is dedicated to mindless catching, logging and rethrowing of exceptions. And we don't see any specific use case or community where Kotlin's design for exception handing is a major obstacle to its adoption.

Of course, Kotlin is not perfect, and of course feedback is welcome. To me, it looks like the best way to address a problem is through better tools - tools that run separately from the compiler and can have much more fine-grained configuration concerning the types of exceptions for which you want special handing and the APIs for which you want that handling. I believe that a solution can be implemented without any changes to the language, simply by building additional inspections in the IDE.

@abreslav

This comment has been minimized.

Copy link
Member

commented Aug 25, 2017

I don't think it's practical to require all exceptions to be handled/declared at all times in our setting. For one thing, it's not clear how we can work with Java code that does not declare RuntimeExceptions. So, I think the only way we get anything like this in the language is through a compatible extension, not a breaking change.

On a side note, we are now looking into a more general mechanism for effect capturing, and optional checking of exceptions may be a nice use case for that.

@BenLeggiero BenLeggiero force-pushed the BenLeggiero:error-handling branch from 52f5692 to 87ba70c May 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.