New lint suggestion: uncaught_async_error_in_try_catch

[gmpassos]

I suggest the lint uncaught_async_error_in_try_catch (or a better name), for the example bellow:

/// Here the call to [_computeImpl] should trigger a warning,
/// since the `try` block won't catch any error from `_computeImpl`:
Future<int?> compute1(String value) async {
  try {
    return _computeImpl(value) ;
  } catch(_) {
    return null ;
  }
}

/// This is the fixed version, where the `await` ensures
/// that the `try` block will catch errors.
Future<int?> compute2(String value) async {
  try {
    return await _computeImpl(value) ;
  } catch(_) {
    return null ;
  }
}

/// A simple computation that can trigger an [ArgumentError].
Future<int?> _computeImpl(String value) async {
  if (value.isEmpty) {
    throw ArgumentError("Empty `value`");
  }

  return value.length ;
}

Without a lint the issue above is not simple to detect and debug and can generate many bugs.

[eernstg]

Good catch, @gmpassos!

The behavior which is described here is a bug, reported in dart-lang/sdk#44395. So the async error should have been caught.

However, a fix for the bug was created already a couple of years ago, and then it was never landed because it gave rise to subtle breakages. In short, we kept the bug because it seemed more disruptive to fix it.

The issue is (finally) being handled in the coming months, as described in this comment.

It may or may not be justified to create a lint as requested here, given that the issue is gone as soon as we have an implementation of dart-lang/language#870, and the code has been migrated to use that. On the other hand, it certainly won't hurt to have a lint like this. It would nudge developers in the direction of pre-migrating their code to the form that it needs to have anyway when dart-lang/language#870 has been implemented.

[gmpassos]

If, in the future, Dart doesn't allow returning a Future without await inside an async block, it could potentially break a lot of code.

IMHO I consider it a bug only if it's inside a "try" block, as without it (try block), the behavior remains the same with or without await.

It might be challenging to implement this language change, and it could take a while to release it. A lint could help resolve the issue for now, in a simple and compatible way.

Also, consider allowing the return of Future (or anything) without await for a method returning FutureOr to avoid disrupting optimizations, since a method that returns FutureOr usually is attempting to optimize things. See https://pub.dev/packages/async_extension

[eernstg]

@gmpassos, you might want to add this comment to dart-lang/language#870, it seems to be more relevant there.

[gmpassos]

THX. I have just commented there.