What role is runZoneGuarded playing here?

What role is runZoneGuarded playing here?

AFAICT, runZonedGuarded is the only way to catch 100% of asynchronous errors that occur as a side-effect.

For example:

// Will catch errors _returned_ by body, but not as a side-effect of body.
try {
  await body();
} catch (e) {
  print(e);
}

What role is runZoneGuarded playing here?

AFAICT, runZonedGuarded is the only way to catch 100% of asynchronous errors that occur as a side-effect.

For example:

// Will catch errors _returned_ by body, but not as a side-effect of body.
try {
  await body();
} catch (e) {
  print(e);
}

right. To put it another way, as far as I understand, runZonedGuarded() will create a new zone, run a body callback within that zone, and then catch all uncaught errors thrown from that zone.

So this is super confusing and easy to get wrong, but it does actually make sense :).

The entire purpose of this API is to swallow errors, it does not allow them to escape the Zone. This includes async errors.

So, if there is an error attached to a Future which escapes the Zone (such as being returned from the body callback), that future will never complete, because it isn't allowed to complete with an error (outside the zone), and has no success value to complete with either. Instead, onError is called and that is it.

Fwiw I thought we added a different API which does essentially what your fix is, and has a return type of R instead of R?, with a required onError which has a return type of R. But I can't find it now. @natebosch am I making this up?

@jakemac53 fwiw our general discontent around this is largely that Zones are basically entirely undocumented, so it's all guesswork when we try to use them. For example, dart-lang/sdk#26455, dart-lang/sdk#54177, dart-lang/sdk#17667, dart-lang/sdk#44669, dart-lang/sdk#26454, ...

Sure, I have felt that pain many times before :). Even where there are docs I find them incredibly difficult to understand. I think the number of people who would self-proclaim to understand zones fully is 2, possibly just 1. I am not one of them :).

But, for this particular issue, I hope the simplified explanation I provided above helps illuminate what is actually happening here?

I'm trying to encourage you to explain zones by submitting CLs to improve the API docs rather than just commenting on PRs. :-)

Fwiw I thought we added a different API which does essentially what your fix is, and has a return type of R instead of R?, with a required onError which has a return type of R. But I can't find it now. @natebosch am I making this up?

I think you might be conflating another API. Future.catchError expects the error handler to return value with the same type as the Future but does not express this in the signature, and this caused some difficulty during the null safety migration. When we introduced FutureExtensions.onError we kept the same expectation but it's now checked statically.

We don't have any API that expects a value returned from a Zone error handler. There is no guarantee that this error happens at a time when it would be useful to complete a Future returned from the runZonedGuarded call - for example the following has both an early zone error and a later real value returned from the callback.

import 'dart:async';

void main() async {
  final value = await runZonedGuarded(() {
    Future.error('Some error'); // Not awaited - unhandled async error.
    return Future.delayed(const Duration(milliseconds: 500), () {
      return 'Some value';
    });
  }, (e, st) {
    print('Unhandled async error: $e\n$st');
  });
  print('Returned value: $value');
}

@lrhn - WDYT about adding an API in package:async for this? The Result class might be a good place to stick it - that class is already dealing with treating errors as values. Maybe something like

  /// Runs [callback] in a zone and returns the result or the first
  /// unhandled error that occurs in the zone.
  ///
  /// If an unhandled asynchronous error occurs before the Future
  /// returned from [callback] has completed, the callback result
  /// will be ignored (if it ever completes), and the returned Future
  /// will complete with the same error.
  ///
  /// If there are multiple unhandled asynchronous errors, or if the
  /// Future returned by [callback] has an error after an unhandled
  /// asynchronous error, all but the first occurring error is ignored.
  ///
  /// This API may be useful to avoid Futures that never complete
  /// due to an error that cannot escape it's error zone.
  Future<T> Result.runZonedSmuggleErrors(Future<T> Function() callback);

Maybe we can also have a callback to handle those subsequent errors instead of strictly ignoring them.

Yeah I think that is really the API most people are expecting here.