catchError and onError do not catch error with function without "async" keyword

[kaboc]

Future<void> main() async {
  final result = await calcOrThrow(-10).onError((e, s) => 0);
  print(result);
}

Future<int> calcOrThrow(int value) { // No "async" keyword here
  if (value < 0) {
    throw Exception('ERROR');
  }
  return calc(value);
}

Future<int> calc(int value) async {
  return value * 2;
}

Unhandled exception:
Exception: ERROR

It occurred only with some functions, so I compared with other functions and found that it depended on whether a function had the "async" keyword. If I add async to calcOrThrow() above, onError() can catch the exception thrown there.

Future<int> calcOrThrow(int value) async {

However, it is possible to omit async because calcOrThrow() directly returns the future returned from calc(). The analyzer warns nothing about it, so I think this misuse (or a bug?) can easily happen.

It is also confusing that try-catch worked fine regardless of async.

Future<void> main() async {
  try {
    final result = await calcOrThrow(-10);
    print(result);
  } on Exception {
    print(0);
  }
}

Future<int> calcOrThrow(int value) { // No "async" keyword here
  ...
}

It seems this issue exists in whenComplete() and the onError argument of then() too.

[lrhn]

This is expected behavior.

The onError method on Future only gets called if calcOrThrow returns a Future. It doesn't if it throws instead.

The try/catch works because it catches any error thrown by the try block. If calcOrThrow returns a future with an error, then the await is what makes that an error thrown in the try block. Directly throwing also works.

We generally recommend that asynchronous functions (those returning a Future, not just those with the async keyword, I'll refer to those as async functions instead) do not throw synchronously, precisely because it means the caller needs to layers of error handling.

What you can do is to either change calcOrThrow to being async:

Future<int> calcOrThrow(int value) async {
  if (value < 0) {
    throw Exception('ERROR');
  }
  return await calc(value);
}

or to make it throw "asynchronously" by returning an error future:

Future<int> calcOrThrow(int value) { // No "async" keyword here
  if (value < 0) {
    return Future.error(Exception('ERROR'));
  }
  return calc(value);
}

[kaboc]

@lrhn
Thank you for the explanation. I understand it, but it seems unsafe that we cannot know if the function is asynchronous without seeing its definition and that we have to be really careful not to omit "async" or to make sure everything is returned asynchronously. The IDE just tells me the return type is Future, whether the function is asynchronous or not.

Even if this is expected behaviour, I think it is error-prone. Now that I know onError is a bit dangerous to use, I'll probably avoid using it and use try / catch instead. I hope a lint rule will be added to warn about the misuse.

[lrhn]

Asynchronous functions (and Future itself) existed before the async/await syntax was introduced. That is one of the reasons it's possible to write functions without async that still return a Future. (Another reason is that it's necessary, because not all primitive asynchronous operations can be expressed using await, for example Future.wait can't.)

So we can't just disallow unsafe code.

The lint to disallow asynchronous operations in non-async functions is called discarded_futures (which is arguably a slightly misleading name, since it triggers whether you discard the future or not, as long as you use it in a non async function).
That lint is not enabled by default because there is both a lot of perfectly good legacy code which doesn't use async/await, and reasonable now uses of it too.
It's probably a fine lint to enable for application code, and less viable for library/framework code, which needs to work directly with asynchrony and futures.

We could consider a lint to prevent (obvious) synchronous throwing in non-async functions that return a Future.

[kaboc]

@lrhn
I tried enabling discarded_futures (that I thought was already enabled, but was actually missing). Unfortunately, I still saw no warning. It warns if I change the function to return non-future, but then the result of calc cannot be returned.

int calcOrThrow(int value) {
  if (value < 0) {
    throw Exception('ERROR');
  }
  calc(value); // lint

  // return calc(value); // error (A value of type 'Future<int>' can't be returned from ...)
}

We could consider a lint to prevent (obvious) synchronous throwing in non-async functions that return a Future.

Hope it will be added. Thank you!