dart-lang / language

Design of the Dart language
Other
2.65k stars 203 forks source link

Add checked exceptions. #984

Closed bsutton closed 3 years ago

bsutton commented 4 years ago

It has to be said...

The one thing I miss moving from Java to Dart is the lack of checked exceptions.

It makes it really hard to put complete error handling mechanisms in place if you don't know the full set of exceptions that a method can throw.

I find it rather ironic that dart doesn't have checked exceptions whilst 'Effective Dart' lints generate an error essentially saying 'check your exceptions'.

try {
      Directory(path).createSync(recursive: recursive);
    } 
    catch (e) {
      throw CreateDirException(
          'Unable to create the directory ${absolute(path)}. Error: ${e}');
    }

generates the following lint: Avoid catches without on clauses.

To fix this lint I need to read source code to discover the exceptions that are going to be thrown.

Exceptions were created to stop people ignoring error. Unchecked exceptions encourages people to ignore errors.

Have we learnt nothing?

I know this probably won't get anywhere due to the current religious movement against checked exceptions and the disruption to the dart eco system but this implementation decision was simply a bad idea.

julemand101 commented 4 years ago

I do agree but you should properly move this issue to the language project: https://github.com/dart-lang/language/issues since the language itself does not support checked exceptions.

lrhn commented 4 years ago

Most languages created after Java has learned from Java and not introduced checked exceptions. They sound great in theory, but in practice they are (rightfully or not) vilified by the people having to maintain the throws clauses. I think the general opinion is that it's technically a good feature, but it doesn't carry its own weight.

I don't see Dart going that way at the current time.

To fix this lint I need to read source code to discover the exceptions that are going to be thrown.

You can write on Object catch (e) if you just want to satisfy the lint. Or you can drop the lint. If you actually want to catch the exception being thrown (and you should when it's an exception, not an Error), then the method documentation should document it clearly.

The standard way to document exceptions is a paragraph starting with "Throws ...". I admit that not all code follows that standard, and dart:io suffers from being written before most standards were formed.

bsutton commented 4 years ago

I admit that not all code follows that standard

And this is exactly the problem.

I would argue that checked exceptions more than carry their weight. With modern IDE's the overhead of managing checked exceptions is largely automated.

The lesson that doesn't seem to have been learned is that developers are lazy and inconsistent (myself included) and if we don't force them to directly address errors then they simply ignore them and the quality of software suffers as a result.

I use a fair amount of flutter plugins and largely there is simply no documentation on what errors can be generated. Dart actually makes it harder to document errors as a developer now actually has to write documentation. Checked errors force devs to document their errors and actually make it easier as the IDE inserts them.

The end result is that for a large chunk of the code base we use on a daily basis, errors are simply not documented and we have to do additional testing and investigation to determine what errors need to be dealt with.

In the business world they have the concept of 'opportunity cost' which is essentially if I invest in 'A', I can't invest in 'B'. What is the cost not doing 'B'? That is the opportunity cost.

With unchecked exceptions we wear the cost of maintaining the checked exceptions but we don't have to wear the cost of investigating what errors can be generated nor the debugging time spent because we didn't handle an exception in the first place.

If a library maintainer has to spend time to declare the checked exceptions that time is more than offset by developers that use that library not having to investigate/test for what errors will be generated.

I believe the backlash against checked exception is because most developers prefer to ignore errors and checked exceptions require them to manage them.

munificent commented 4 years ago

I think if a function's failure values are so important to be handled that you want static checking for them, then they should part of the function's return type and not an exception. If you use sum types or some other mechanism to plumb both success and failure values through the normal return mechanism of the function, then you get all of the nice static checking you want from checked exceptions.

More pragmatically, it's not clear to me how checked exceptions interact with higher-order functions. If I pass a callback to List.map() that can throw an exception, how does that fact get propagated through the type of map() to the surrounding caller?

You describe the movement away from checked exceptions as "religious", but I think that's an easy adjective to grab when you disagree with the majority. Is there a "religious movement" towards hand washing right now, or is it actually that the majority is right and hand washing is objectively a good idea? If almost everyone doesn't like checked exceptions, that seems like good data that it's probably not a good feature.

most developers prefer to ignore errors and checked exceptions require them to manage them.

You're probably right. But if you assume most developers are reasonable people, then that implies that it should be relatively easy to ignore errors. If it harmed them to do it, they wouldn't do it. Obviously, individuals make dumb short decisions all the time, but at scale I think you have to assume the software industry is smart enough to not adopt practices that cause themselves massive suffering.

bsutton commented 4 years ago

"I think if a function's failure values are so important to be handled that you want static checking for them, then they should part of the function's return type and not an exception."

You must be younger than me :)

Exceptions were introduced to solve the failings of return types. 1) having to pass return types up the call stack. 2) using the return to indicate error conditions means that you can't use the return type to pass the 'good path' values. 3) poor documentation 4) devs choosing to ignore error codes.

Before checked exceptions, ignoring error returns was the norm not the exception (pun possibly intended).

I'm involved in the usage and maintenance of a number of open source dart packages and I'm seeing the same problem again. Code that ignores errors is almost the norm rather than the exception. A complete lack of documentation on the errors that methods return.

I've been adding effective dart lints to a number of packages lately and one of the lints is to use an 'on' clause in the catch block. I've mostly had to suppress the lint as I would have literally had to read through thousands of lines of code to find out what exceptions can be thrown. I never have this problem with java. I can always work out exactly what exceptions I need to deal with and if you read my code you can easily determine what error conditions you have to handle.

You identify a religion by a pattern of behaviour that defies logic. The lack of checked exceptions has resulted in a proliferation of packages that poorly handle errors and applications that do the same.

Do a survey of pub.dev and the evidence is all over the place.

Google seems to take a fairly statistical approach to language changes.

I would advocate that Google do an analysis on the level of documentation for errors in pub.dev.

I suspect the stats will be damning.

Brett

On Fri, 29 May 2020 at 09:47, Bob Nystrom notifications@github.com wrote:

I think if a function's failure values are so important to be handled that you want static checking for them, then they should part of the function's return type and not an exception. If you use sum types or some other mechanism to plumb both success and failure values through the normal return mechanism of the function, then you get all of the nice static checking you want from checked exceptions.

More pragmatically, it's not clear to me how checked exceptions interact with higher-order functions. If I pass a callback to List.map() that can throw an exception, how does that fact get propagated through the type of map() to the surrounding caller?

You describe the movement away from checked exceptions as "religious", but I think that's an easy adjective to grab when you disagree with the majority. Is there a "religious movement" towards hand washing right now, or is it actually that the majority is right and hand washing is objectively a good idea? If almost everyone doesn't like checked exceptions, that seems like good data that it's probably not a good feature.

most developers prefer to ignore errors and checked exceptions require them to manage them.

You're probably right. But if you assume most developers are reasonable people, then that implies that it should be relatively easy to ignore errors. If it harmed them to do it, they wouldn't do it. Obviously, individuals make dumb short decisions all the time, but at scale I think you have to assume the software industry is smart enough to not adopt practices that cause themselves massive suffering.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dart-lang/language/issues/984#issuecomment-635675501, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG32OGKTAU2HOQKVSMIPBLRT3ZZVANCNFSM4NKANDJQ .

bsutton commented 4 years ago

Just a final thought.

Why would you move from a system (checked exceptions) which automates the documentation of code to one that requires developers to document their code.

The empirical evidence (again look at pub.dev) is that developers do a terrible job of documenting and view documentation as a burden. Managing checked exceptions is a far smaller burden than documenting code.

The lack of checked exception really has to be the best example of developers shooting themselves in the foot :)

leafpetersen commented 4 years ago

Why would you move from a system (checked exceptions) which automates the documentation of code to one that requires developers to document their code.

My general take on this is that effect type systems (of which checked exceptions is one) tend to interact badly with higher-order code, and modern languages including Dart have leaned in heavily to using first class functions in libraries etc. Indeed, the first search result I got when I just looked to see what Java does with this these days was this, which isn't inspiring. There may be better technology for making this non-clunky now, but last I looked at it, it took some pretty hairy technology to be able to express higher-order functions that were parametric in the set of exceptions that could be thrown. Inference can help with that, but as the saying goes, now you have two problems... :)

The empirical evidence (again look at pub.dev) is that developers do a terrible job of documenting and view documentation as a burden.

I will admit that it personally bothers me a lot that even for well-documented Dart team owned code, I often have to go poke around in the implementation to figure out whether an exception can be thrown, and in what circumstances.

rrousselGit commented 4 years ago

Exceptions were introduced to solve the failings of return types. 1) having to pass return types up the call stack. 2) using the return to indicate error conditions means that you can't use the return type to pass the 'good path' values. 3) poor documentation 4) devs choosing to ignore error codes.

Exceptions are not the only way to solve these. There are multiple languages out there with no exception mechanism at all and that do just fine.

The alternative is usually a combination of union-types and tuples and destructuring. This leads to self-documenting code, where it's impossible to ignore errors, while still being able to return valid values

For example using unions, instead of:

int divide(int value, int by) {
  if (by == 0) {
    throw IntegerDivisionByZeroException();
  }
  return value / by;
}

void main() {
  try {
    print(divide(42, 2));
  } on IntegerDivisionByZeroException catch (err) {
    print('oops $err');
  }
}

we'd have:

IntegerDivisionByZeroException | int divide(int value, int by) {
  if (by == 0) {
    return IntegerDivisionByZeroException();
  }
  return value / by;
}

void main() {
  switch (divide(42, 2)) {
    case IntegerDivisionByZeroException (error) => print('oops $error'),
    case int (value) => print(value);
  }
}
bsutton commented 4 years ago

I'm not certain unions result in more readable code than catch blocks. If I'm reading the code correctly it does provide a level of documentation but it appears that it will still allow the caller to ignore the error and fail to pass it back up. So once again we are dependant on the developer to do the correct thing and we are stuck with undocumented code.

Tuples will have the same issues.

bsutton commented 4 years ago

My general take on this is that effect type systems (of which checked exceptions is one) tend to interact badly with higher-order code, and modern languages including Dart have leaned in heavily to using first class functions in libraries etc

This appears to be a broader problem with how do you handle errors in lambda etc. Whether its a checked/unchecked exception, error, union or tuple you still need to handle the errors in the top level function. Error returns simply allow you to (incorrectly) ignore the error. Your code looks nice, but does it actually behaviour correctly?

rrousselGit commented 4 years ago

Error returns simply allow you to (incorrectly) ignore the error.

That is not the case. Unions forces you to check all possible cases, or it is otherwise a compilation error.

Continuing with the code I gave previously, it would be impossible to write:

int value = divide(42, 2); // IntegerDivisionByZeroException is not assignable to int

You would have to either cast the value or check the IntegerDivisionByZeroException case.

This is in a way non-nullable types, but broadened to apply to more use cases

bsutton commented 4 years ago

OK, sure.

There however seems little point to introducing a new mechanism when we already have exceptions in the language. We could possible even start by requiring checked exceptions via a lint.

We need some language experts to comment on this.

leafpetersen commented 4 years ago

We need some language experts to comment on this.

Touché. I'm afraid we're the best you're going to get though - Google can't afford to hire better.

bsutton commented 4 years ago

Sorry, no insult was intended:)

On Sat, 30 May 2020, 11:13 am Leaf Petersen, notifications@github.com wrote:

We need some language experts to comment on this.

Touché. I'm afraid we're the best you're going to get though - Google can't afford to hire better.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dart-lang/language/issues/984#issuecomment-636252762, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG32OE6BNZ7C5ZWAU4DZ5TRUBMUNANCNFSM4NKANDJQ .

bsutton commented 4 years ago

So I come back to my evidenced based approach for this feature:

I would advocate that Google do an analysis on the level of documentation for errors in pub.dev.

If it turns out that errors are well documented then I will withdraw my feature request.

Let's now find out if decisions on checked exceptions are driven by religion or science.

munificent commented 4 years ago

So I come back to my evidenced based approach for this feature:

I think the clearest, easiest-to-acquire evidence is that almost no users are requesting this feature despite ample evidence of its existence thanks to Java. Given that, I think the burden of proof is on those requesting it to show that it's a good idea. If the language team had to actively disprove every feature request, we'd never make any progress. :)

bsutton commented 4 years ago

So what would be an acceptable metric?

munificent commented 4 years ago

Pragmatically, I can't see what metric would be persuasive here. Even if we were all 100% sold on checked exceptions, adding them would be a massively breaking change to the language. We have too many users and too many lines of code in the wild. In order to justify a migration of that scale, we'd want to see hordes of users beating down the door demanding it (which is what we do see, for example, for non-nullable types). I just don't see the user demand for this, at all. And, ultimately, we are obliged to make a thing that users want.

bsutton commented 4 years ago

@munificent yes I do see that as a problem.

The 'somewhat vague' thought was this could be introduce via a lint so like types it is optional.

Sometimes you need to give people what they need rather than what the want :)

leafpetersen commented 4 years ago

Pragmatically, I can't see what metric would be persuasive here. Even if we were all 100% sold on checked exceptions, adding them would be a massively breaking change to the language

In addition, I'd also add that it's not enough to have a metric that says there is a problem. You also need to be able to argue that a proposed solution is a good one. My objection (and I think most of the objections above) is not to the characterization of the problem, but to the solution. I think that checked exceptions are not a good solution to the problem, for reasons that I touched on above, and that is one of the reasons they have not become more widespread outside of Java. These kind of effect systems don't easily compose with existing language features (like generics and higher order functions). While encoding errors into return types can be a bit heavy, it has the great advantage that it is an approach which composes well with already existing (and otherwise useful) language features.

bsutton commented 4 years ago

Encoding errors into return types has already been proven not to work which is why C++ had exceptions to fix the problem of C.

Encoding errors into return types simply results in users ignoring the errors which is the problem that dart has now due to the lack of checked exceptions.

I would argue that check exceptions is the best solution we have to the problem. Its just that developers are inherently lazy and don't like being forced to deal with errors.

The pub.dev code base demonstrates this in spades.

Whilst we do need to listen to the user base, we also need to ensure the health of the eco system and sometimes that means administering some medicine, even if it tastes awful.

leafpetersen commented 4 years ago

Encoding errors into return types has already been proven not to work which is why C++ had exceptions to fix the problem of C.

There are many more modern languages which use this approach, either in it's monadic formulation, or directly, than which use checked exceptions, so I'm going to have to disagree with your characterization of the relative success of these approaches.

That said, I don't claim it's a wonderful solution. My take on the space is as follows:

I understand that the "not great" solution doesn't help enforce hygiene for the ecosystem, which is why I say it's not great.

I would argue that check exceptions is the best solution we have to the problem.

That may be, but it is still a bad solution. As far as I can tell, the best advice out there for working with higher order functions in Java is to take every single lambda, wrap its body in a try/catch which catches the checked exceptions that may be thrown in it, and rethrow an unchecked exception. Am I wrong? That helps nobody, and makes for a completely unusable language.

bsutton commented 4 years ago

As far as I can tell, the best advice out there for working with higher order functions in Java is to take every single lambda, wrap its body in a try/catch which catches the checked exceptions that may be thrown in it, and rethrow an unchecked exception. Am I wrong? That helps nobody, and makes for a completely unusable language.

This is the crux of the problem.

You are essentially arguing that error handling makes the code look ugly so you would prefer to ignore the errors.

Whether the errors are a checked exceptions or a return type, if an error is going to be returned the lambda MUST handle the error.

Ignoring errors to make the code look nice is not an acceptable argument.

I too would prefer a more elegant solution to handling errors in lambda's but you are blaming the messenger (checked exceptions) for the problem.

The issues is really with lambdas not with checked exceptions.

Perhaps we need to look at how futures handle exceptions and whether some similar concepts could be applied to lambdas.

leafpetersen commented 4 years ago

You are essentially arguing that error handling makes the code look ugly so you would prefer to ignore the errors.

Whether the errors are a checked exceptions or a return type, if an error is going to be returned the lambda MUST handle the error.

No! You are deeply misunderstanding the problem, and this gets at the crux of it! :)

The lambda is not the correct place to handle the problem. The correct place to handle the problem is higher up the stack. This can be clearly seen in the advice I quote above (which really is all over the internet - look for it)! The advice is not catch the error and deal with it the error is catch the error and throw an unchecked exception because the lambda is the wrong place to deal with the error. So the result is you get neither safety (your lambdas just catch and discard errors) nor brevity (you must explicitly catch and discard errors).

To make this actually work properly, you implement a full effect system, which lets your higher-order functions be parametric over the implied effects of their arguments. So your .map method is parametric over the set of exceptions possibly thrown by its argument. This works out ok, sort of, but it's terrible to work with, and an extremely heavyweight feature.

So you instead you get the mess that is Java checked exceptions, which, as I say, doesn't compose with higher order functions.

If you encode errors in the return type (e.g. using monads), then it's entirely up to the programmer where to handle the error. The lambda simply gets type int -> OrError(int) instead of int -> int, mapping gives you an Iterable<OrError(int)> and now to get at the contents, you need to explicitly iterate through either discarding the errors (nothing stops you from hitting yourself if you really want to), accumulating them (e.g. with a monadic fold), or handling them. But the point is that the programmer can choose at what level of the stack to handle the error.

I'm not saying it's ideal (it's not!) but it is strictly better than checked exceptions as implemented in Java, and it is entirely expressible in Dart as it exists today.

Perhaps we need to look at how futures handle exceptions and whether some similar concepts could be applied to lambdas.

Yes! We do need to! And ... it's... a ... wait ... for ... it.... monad! Futures are a monad, and they reify exceptions into the monadic value, so that the exception can be handled (or not handled) by the end user by binding the error and handling, rather than throwing an asynchronous exception from somewhere deep in the bowels of the runtime system (usually).

simophin commented 4 years ago

Checked exception:

  1. Is infectious, from bottom to top;
  2. Because its infectiousness, developers will be encouraged to either: a. Swallow the error. b. Convert it to RuntimeException.

Both of which defeat the purpose of a checked exception. These actions taken by developers are not rare, you see it everywhere! What ironic it is: that a feature designed to help write better code, will encourage you to ignore it?

If you talk about documentation purpose, surely a union type should be sufficient?

OlegAlexander commented 3 years ago

An excellent discussion so far. I agree with @leafpetersen that adding checked exceptions to the language may not be the best solution to this problem because as @simophin pointed out, checked exceptions will not magically turn bad programmers into good ones. However, there is a problem here, especially when it comes to the documentation of exceptions. As you will see, the problem could be at least partially solved with some changes to dartdoc.

I've recently run into this issue with the glob package. Before I go on, I should say that glob is an excellent package! I'm only using it as an example to discuss potential improvements to the documentation of exceptions in Dart.

This line will throw a StringScannerException defined in the string_scanner package, which is a dependency of glob.

final invalidPattern = '';
final glob = Glob(invalidPattern); // Throws StringScannerException 

The 1.2.0 Glob constructor documentation doesn't mention the StringScannerException. Okay. But what if I wanted to document that my function, which uses Glob may throw a StringScannerException? Simply saying /// Throws [StringScannerException] at the top of my function doesn't work, because it seems dartdoc can only link to identifiers that are visible in the current file. So I have to first add string_scanner as a dependency in my pubspec.yaml and then import 'package:string_scanner/string_scanner.dart'; into the current file. Now dartdoc will recognize the link to [StringScannerException].

Having to directly depend on a transitive dependency for documentation purposes is not ideal. Also, a package you depend on may add/remove exceptions without warning, which may make your own documentation (and code!) out of date.

It would be better if dartdoc could somehow automatically enumerate all the possible exceptions a function may throw (except for the ubiquitous ones, like OutOfMemoryError.) In other words, the exceptions should be a part of the function signature, but this doesn't necessarily need to happen in the language. It just needs to happen...somehow 🙂

Thoughts?

bsutton commented 3 years ago

I've raised an issue suggesting that we add a lint rule as a middle ground.

The lint rule would give most of the benefits in that exceptions would be documented without requiring a fairly significant changed to the language.

https://github.com/dart-lang/linter/issues/2246

lrhn commented 3 years ago

For what it's worth, that's actually design smell from the Glob package that its public API exposes a class from a different package, one which it doesn't export itself. I'd prefer if it caught the string scanner's exception and threw a FormatException instead. If the analyzer had warned about the undocumented exception, then it's probably more likely that the package would have noticed and handled it. (Edit: And they apparently do throw FormatException since StringScannerException implements it. It would still be better if that was documented in the public API.)

OlegAlexander commented 3 years ago

Thank you, @lrhn, for your insight. Upon digging into the string_scanner code further, I discovered that StringScannerException ultimately implements FormatException! So thankfully, I don't have to directly depend on string_scanner anymore just for the sake of a dartdoc link.

I'll admit that I'm a relative newcomer to Dart and that maybe I could've found this solution sooner. But I think the main point is still valid because I only found out about it after considering your (original unedited) response and then reading the code.

I like your idea about the analyzer warning about undocumented exceptions. I think it would have a major impact on the quality of documentation on pub.dev. Is anyone else in favor of this idea?

bsutton commented 3 years ago

@OlegAlexander

Add a vote to this issue will help to move the discussion forward.

https://github.com/dart-lang/linter/issues/2246

weenzeel commented 3 years ago

I'm new to Dart and came to this thread after having ben caught by surprise by an

Unhandled exception:
FileSystemException: Directory listing failed, path = '/Users/.../' (OS Error: No such file or directory, errno = 2)
#0      _Directory._fillWithDirectoryListing (dart:io-patch/directory_patch.dart:37:68)
#1      _Directory.listSync (dart:io/directory_impl.dart:243:5)
#2      main
#3      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:299:32)
#4      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168:12)

I made a typo and a the machine didn't find the directory that I swore would be at the path I provided. :)

As I said, I'm new to Dart and didn't know that "In contrast to Java, all of Dart’s exceptions are unchecked exceptions. Methods do not declare which exceptions they might throw, and you are not required to catch any exceptions."

What is expected of as a developer in Dart to write safe code for this (and other) methods?

The documentation for dart:io dosen't mention any exceptions:

/**
   * Lists the sub-directories and files of this [Directory].
   * Optionally recurses into sub-directories.
   *
   * If [followLinks] is false, then any symbolic links found
   * are reported as [Link] objects, rather than as directories or files,
   * and are not recursed into.
   *
   * If [followLinks] is true, then working links are reported as
   * directories or files, depending on
   * their type, and links to directories are recursed into.
   * Broken links are reported as [Link] objects.
   * If a link makes a loop in the file system, then a recursive
   * listing will not follow a link twice in the
   * same recursive descent, but will report it as a [Link]
   * the second time it is seen.
   *
   * Returns a [List] containing [FileSystemEntity] objects for the
   * directories, files, and links.
   */

Following the code down the rabbit hole leads to this definition of an external method without mentioning any of the exceptions that may be thrown inside it.

  external static void _fillWithDirectoryListing(
      _Namespace namespace,
      List<FileSystemEntity> list,
      Uint8List rawPath,
      bool recursive,
      bool followLinks);

How am I suppose to know when to wrap a statement in a try block? How do I discover the types that the code may throw at me as I use it?

Thanks!

lrhn commented 3 years ago

What is expected of as a developer in Dart to write safe code for this (and other) methods?

Read the documentation and handle any exceptions it states that it might throw.

The documentation for dart:io doesn't mention any exceptions:

Well, ouch! :blush:

The dart:io library could do with an overhaul. It was one of the first libraries written after introduction of futures and streams, and one of the first complicated APIs we had. The documentation (and implementation) isn't always up to modern standards, and we should do something about that.

weenzeel commented 3 years ago

Ouch in deed!

The only language I know well is Swift and for what it's worth I think it strikes the right balance regarding error handling, and would feel like idiomatic Dart now that null safety is coming.

If you want to throw you must mark your method as a throwing method. You can only throw an object that conforms to the Error protocol.

Any call to a throwing method must be marked with a try keyword (to propagate or handle the error), try? (to turn the call in to an optional value) or try! (to disable error handling).

If you want to handle an error you can use do-catch clauses.

There is no need to handle specifik errors if you don't want to. But someone, somewhere needs to handle at least the basic error type for throwing functions in some way or the error will crash the app.

If you provide a custom Error type as an enum the compiler can help you to make sure that you handle all of the cases, even if that just means providing a default catch clause.

In Swift the language helps to bring my attention to code that may throw. And I know how to handle it at least for the most generic case (catch an object of type Error) without looking at the documentation. If library code implements a more nuanced error taxonomy this needs to be documented by the library author or discovered by the library user. But this is a good compromise to me, offering safety for everyone, but allowing for increased complexity when needed.

The argument above that we're in the wild with a lot of code for a breaking change like this is of course valid. But so is the argument that there are a lot of code out there with exceptions waiting to happen that no one knew about when the code was written. As I proved 1h ago. Thank god someones life didn't depend on it (this time ;).

Swift Error Handling

cedvdb commented 3 years ago

What is expected of as a developer in Dart to write safe code for this (and other) methods?

Read the documentation and handle any exceptions it states that it might throw.

It's not documented well, maybe in dart libs, but not in packages in pub dev.. A "throws" keyword would be nice to specify the error types the user should be worried about, if only for abstract classes so their implementation can throw specific things. Else it can be part of the result. but I find it weird.

class DivisionResult {
   final bool success;
   final double result; 
}
nyarian commented 3 years ago

checked exceptions have a poor reputation due to their misuse - once a checked, but overly generic exception leaks into the public API, then it can become increasingly annoying to handle it if you are forced to (tribute to JDBC's SqlException)

a good use for them, though, can be depicted in the context of domain modelling as a mean for declaring expected, high-level exceptional cases that are required to be handled. just a simple example:

class User {
    Purchase buy(Product product) throws InsufficientMoneyException { ... }
}

to extend the snippet, I see only one alternative that will enforce complete error handling: creating an abstract PurchaseException type, creating a set of subtypes that fall under this category (InsufficientFundsException would be one of them), creating a visitor-based interface (to force to handle all the exceptions; sadly, we currently do not have pattern matching that possibly would help us to force iterating over all the possible cases of a sum type, and instance checks won't signal us about a newly added breach when we add one additional exceptional case as a subtype), and making the function itself to return Either<PurchaseException, Purchase>. so, like this:

abstract class PurchaseExceptionVisitor {
    // no overloading, so sorry for the long method name
    void visitInsufficientFundsException(InsufficientFundsException exception);

    // ... other methods for other exceptional cases
}

abstract class PurchaseException implements Exception {
    void visit(PurchaseExceptionVisitor visitor);
}

class InsufficientFundsException implements PurchaseException {
    @override
    void visit(PurchaseExceptionVisitor visitor) => visitor.visitInsufficientFundsException(this);
}

it is quite cumbersome, but it might suffice if it does the complete job. unfortunately, there is an another problem: interference with the Dart's exception propagation system itself. what if something unexpected comes out and error or exception gets thrown out from the method? we would be forced to handle both the Left branch as Either's result as well as catch-ing other, unexpected (unchecked?) exceptions.

we could, of course, go completely rogue, and create a custom abstraction over the function's result (like abstract class PurchaseResult), which would be implemented by subtypes like PurchaseSuccess, InsufficiendFunds, etc. . but you can already see where this is going.

so far checked exceptions seem to be something that can't be easily substituted with monads (FP basically shifts the whole conversation a bit off, as we would be arguing about a way of enforcing a function to not evaluate to bottom in any case, like c++ noexcept or something, but most guys use a mix of procedural and object-oriented paradigms), as there is currently no reliable way to enforce proper handling of a logically enforced sum type

OlegAlexander commented 3 years ago

There's a very similar discussion happening on the Kotlin forum here. There are great arguments for and against checked exceptions there. Here are my takeaways from their discussion (and our discussion here) so far. In my opinion, the viable solutions to not having checked exceptions are as follows:

Other "older" alternatives, like returning error codes or tuples, probably wouldn't catch on in the Dart community. (Though there's nothing wrong with these methods when used properly.)

So my question to the language designers is: What is the idiomatic way to return expected API errors in Dart today?

fredgrott commented 3 years ago

Thanks, @OlegAlexander as I was looking for a way to handle how to grab errors on functions that might be as easy as doing the classic log-mixin stuff on classes. So it seems that your suggestion of a catch-all logic to log both expected and unexpected errors in the main function of both dart and flutter apps is the best solution for now as it reduces any extra logging statements I have to write and yet know that I have something in-place to always catch the expected and unexpected errors.

OlegAlexander commented 3 years ago

There seem to be at least two camps on this issue: Those who want to force the developer to deal with the errors (checked exceptions or sum types) and those who only care that the errors are automatically documented. I belong to the latter camp. It doesn't matter to me what mechanism is used for error handling. If Dart removed exceptions tomorrow and replaced them with something else, it wouldn't matter to me. What does matter to me is that all the errors, especially the expected ones, are documented. This way I know what to expect and I have the option to deal with the errors.

@bsutton's proposal, if implemented, would certainly solve the issue for someone in the "documentation" camp.

In other words, whatever camp you belong to, there's simply no universe in which automatic documentation of errors in Dart is a "bad" thing.

munificent commented 3 years ago

What is the idiomatic way to return expected API errors in Dart today?

"Expected" and "error" make for strange bedfellows. If it's so expected that you want to force users to handle it, how much is it really an "error"? In practice, I think most Dart users:

I'm going to close this issue because I can't see us adding anything like Java's checked exceptions to Dart. While it clearly has some fans, it is broadly disliked and almost no language after Java features them. More importantly, it's not clear to me at all that they can be gracefully incorporated into higher-order functions without adding a lot of complexity to the type system.

At a high level, I think we are all on board with wanting better static checking of erroneous API results, but I don't think checked exceptions are the way to get them. Instead, I think a better approach is to go towards pattern matching and algebraic datatype style.

lukepighetti commented 3 years ago

I'm having a hard time understanding why checked exceptions are anything but value add. I'll give an example.

Given a method with the sum types in format < success : error >

<int : FooException | BarException | BazException> myIntReturningThriceThrowingMethod();
try {
  final result = myIntReturningThriceThrowingMethod();
} on FooException catch(e) {
  print('Foo!');
} /// 👈 analysis server tells us that we need to handle [BarException] and [BazException]
try {
  final result = myIntReturningThriceThrowingMethod();
} on FooException catch(e) {
  print('Foo!');
} catch(e) {
   /// 👆e is type <BarException|BazException>, analysis server is happy
}

Where catch(e) is a greedy collector of errors. Without this greedy collector it would be very terse and a pain to deal with.

Also, without a try block I would expect these error sum types to fall through and sum as they pass through more code.

<int : FooException | BarException | BazException | BiffException> myWrappingMethod(){
  final value = myIntReturningThriceThrowingMethod();

  if (value < 0) 
    throw BiffException();
  else
    return value;
}

Now, there is an argument that this could be handled with union types and a robust where clause. I can buy that, but the important thing is to allow the map/fold/reduce of error types as they are handled and make them explicit. As it stands there is no way to tell which errors might result from calling a method and it provides little incentive (perhaps a little gameified) to handle and reduce errors into something more actionable up the call stack.

DevNico commented 3 years ago

I too don't understand the hate checked exceptions get. A simple use case your points (@munificent) don't cover is what if I have multiple calls to a function that may fail. e.g.

final value1 = await myObject.getSomeValue('value1');
final value2 = await myObject.getSomeValue('value2');
final value3 = await myObject.getSomeValue('value3');
final value4 = await myObject.getSomeValue('value4');

If I value1..4 were some kind of Result object I would have to do something like

if(value1.success && value2.success && value3.success && value4.success) { ... }

The same kind of long and repetitive if has to be written if the function returns null in case of an error.

If the function threw a checked exception I would be informed that I need to handle the exceptions and one try {} catch {} would solve the problem. This syntax would be way more readable and easier to maintain in my opinion.

Maybe instead of adding checked exceptions as a language feature, it could be added as an annotation? If I could declare my function with an @Throws(SomeException.class) annotation and then get analyzer support that way would already be a huge help since one could just configure the rule to act as an error.

Would be great if this could get looked into again!

lukepighetti commented 3 years ago

@mateusfccp I noticed you thumbs downed my comment. Care to weigh in on what I might have missed? What I proposed appears to be only benefit compared to what we have today. Nothing extra to do. But I may have missed something.

munificent commented 3 years ago

I'm having a hard time understanding why checked exceptions are anything but value add.

I look at checked exceptions as essentially an oxymoron. Taking a step back, why have exceptions at all as a language? What is it that they do? Where is the value add?

The really useful thing about exceptions—their actual mechanism—is that they automatically unwind the stack and jump from the point of throw all the way to the point of catch. The obvious nice value is that you don't have to remember to unwind at each point where you don't want to handle the error. You can't silently drop the error on the floor like you can in C.

But it's more than that. It's not just that functions between the throw and catch don't have to manually rethrow the error. It's that they don't have to know anything about them at all. This abstraction and decoupling is really powerful. It means that exceptions can propagate through higher-order functions. It means that an overridden method can throw an exception type that the base class doesn't know about. It means that you can store a closure in a field and call it later from some other function without that second function being coupled to every possible exception that could come out of that closure.

OK, so then what do checked exceptions do? They force any code that could receive an exception to explicitly handle it or rethrow it. Any call frames between where the exception is thrown and where it is caught must explicitly choose to propagate that exception and are now directly coupled to the types of those exceptions.

Doesn't that just cancel out all the value of exceptions in the first place? You've made them no longer exceptions.

Let's say the goal of your API is this:

  1. It produces some kind of value on success.
  2. It produces one of a couple of other different kinds on failure.
  3. You want to ensure that callers handle all possible produced types.
  4. You don't want errors to silently unwind the stack.

What you want here isn't exceptions. You want a return value.

Now, there is an argument that this could be handled with union types and a robust where clause.

Yes, this, exactly. A "failure that you don't want to automatically unwind the call stack" shouldn't be modeled using exceptions because the entire point of exceptions is "automatically unwind the call stack".

lukepighetti commented 3 years ago

That makes sense, however I don't think it takes into consideration that my proposal above allows the consumer to not be forced to handle any exception at all.

No try/catch block? No problem.

No try/catch block and adding a new exception type? That gets added to the signature.

Try catch block? All exceptions are caught and are not added to the signature unless rethrown. That's consistent with our experience today, unless I'm missing something.

You gain the ability to know what errors are coming through the catch. And if you decide to use try/on it can either recommend that you add a catch block to rethrow, provide autocomplete to add an on block for each exception type, or just automatically rethrow anything not explicitly handled.

But if you have a function signature that cannot throw FooException and you wrap it with try / on FooException, it should throw a compiler error that says that's not going to happen.

If my proposal isn't clear I will happily create a more complete document.

lrhn commented 3 years ago

The thing with checked exceptions is that it's an attempt to declare the computation instead of just its result. (We can already declare non-termination using Never, so that's not entirely new).

We'll at least need the ability to abstract over computation. If we allow <int: FooException, BarException> to denote the results of a computation, then I'll want to be able to write something like:

<R:BarException, ...E> handleFoo<R, E*>(<R:FooException,E> Function() f) { // E* is a set of types
  try {
    return f();
  } on FooException catch (e) {
    throw BarException(e);
  }
}

That will make computations first-class abstractable operations, just like we needed generic functions to abstract over the types.

Then all functions taking a callback that they call directly will need to abstract over the exceptions, say:

 <List<T>:...E> List.generate<E*>(int n, <T:...E> compute(int n)) => ...

A callback stored for later usually won't be allowed to throw exceptions, unless there is an outlet for the exceptions.

Which brings us to async code, which is also a problem. We'll want Future to capture exceptions too, to be Future<R, E*> so that await f has type <R: ...E>. We'll want Stream<T, E*> to emit only E exceptions from the await for loop.

If you have to maintain the list of exceptions a method might throw or propagate, it's probably not going to be workable in practice. Java tried, and largely failed, that model. (It's amazing the amount of work people will do to work around the Java checked exception system). So I want a syntax to write <int:> foo() { ... } to infer the exceptions thrown by the body of foo automatically (it's different from just int which means no exceptions). I'd expect most functions to use that syntax, but it's unlikely that we can do that with top-level inference, so it might not work.

Basically, we treat <T, E1... En> as different "return channels", which you trigger by either returning a value or throwing one of E1..En, and the result goes to the surrounding handler for those (which must exist), with the "return" handler being automatically at the call point. In that sense, a return is a throw which is automatically caught at the call point.) We just have t be able to treat all of them abstractly, not just the return value. (I'm toying with a language design where all "return types" are basically union types, and if your function doesn't handle one of the return values of a function it calls, that function just returns out past your function to whoever is handling it. Basically, all returns are throws to the appropriate named handler, the trick is to ensure that there always is a handler. Which brings us back to @munificent's point: Checked exceptions are returns, just ones which are easy to propagate.)

lukepighetti commented 3 years ago

From a purely 'gut check' point of view, it would make sense to me that Errors act the way they do today, but Exceptions be checked. I admit that this could be handled with union types, but I fear that we have so much code where Exceptions are being thrown instead of returned that we'd (in practice) never get to the point where anyone was actually using this pattern. And on top of that, I personally don't feel comfortable having two different levels of 'error.' It's not clear to me if this would be clarifying or confusing.

Edit: I just re-read this and I don't think it's a good idea

ryanheise commented 3 years ago

I'll add my :+1: for this feature. I don't see any insurmountable technical challenges in principle, only surface issues such as the verbosity of the type system, and the implications for code migration. Putting that aside, I think a type system could be designed that fits the criteria, and an exception inference algorithm could be made to help developers automatically add the exception type annotations to their functions through a quick fix. This would also help in the case of higher order functions to ensure that the function's exception type is parameterised. If the concern is that it will affect the surface syntax, I would even settle for making this aspect of the type system completely implicit, but still provide the static analysis tool to be able to report when we have unhandled exceptions in our program. The output of the analyser could be cached per module.

That's my technical perspective, but now for a completely different perspective, here's my marketing perspective:

As the marketing says (and rightfully so), null safety kills bugs brought about by null pointers, one of the most common types of bugs that cause production apps to crash. But you know the other most common cause of bugs: Uncaught exceptions! Without any static analysis tools, it is very difficult to track down and correctly handle all of the exception cases appropriately. The marketing machine has put a very positive spin on the null safety feature (and rightfully so), but of course we know there were also negatives: a huge technical effort behind the scenes, and also a very long and painful migration across the ecosystem. But somehow the pain was worth it, and even though I never asked for this feature, I am glad I now have it. Basically, these two features have similar value and a similar marketing story.

With null safety, I appreciate that I can now change the nullability signature of a method and the analyser will automatically report to me the 15 places in our 15,000 line codebase that will be affected by this change and in fact won't let it compile until all possible crash conditions are handled. But we have exactly the same problem with uncaught exceptions. When we upgrade a 3rd party library and the exceptions thrown by an API changes, we have no way to reliably know how to handle the changes required to our catch blocks. So ultimately we find out after releasing to production.

P.S. I agree with @lukepighetti that we don't want two different language features for the purpose of errors. That would be quite the mess. (Plus, returning an error would be even more verbose because it would require manually unwrapping and re-wrapping the error all the way down the stack all the way to the catcher. True exceptions bypass that and jump straight to the catcher.)

Erhannis commented 3 years ago

Just putting in my vote - as a Java programmer, learning Dart, I miss checked exceptions. (I also write Swift.) I no longer have confidence whether any 3rd party function will return normally. A coworker, who switched from Java to Flutter about a year ago, says basically the absence of checked exceptions are the one thing he hates. I wouldn't mind some other solution - union return types, for example - I just want to have some confidence that my code won't frequently crash for some hitherto invisible reason.

bsutton commented 3 years ago

@Erhannis https://github.com/dart-lang/linter/issues/2246#

I've proposed a lint that would deliver most of the value.

I think it has a better chance of getting support.

If you like the idea then up vote it

lukehutch commented 1 year ago

With proper support for union types (#1222), checked exceptions could be returned as part of the return value (as discussed in several previous comments). For example, int.parse(String) could return type int | FormatException.

Pattern matching (#546) is what would make this much cleaner to work with, for example:

final intVal = int.parse(stringVal) match {
   case int v => v;
   case FormatException e => -1;
}