Giter Club home page Giter Club logo

Comments (4)

lrhn avatar lrhn commented on July 16, 2024

We could change runZonedGuarded to let an error completion escape the zone. At least that will prevent the returned future from never completing if the computation fails.

I think that'll be mostly non-breaking for reasonable uses.
I think we should just do that, since I haven't managed to get rid of the error zone blocking errors entirely.

We could also add a flag that makes it replace a successful completion with an error if there has been any unhandled async error before completion.

I'm less convinced by that, because it seems at odds with how async operations behave.

  • it only triggers if the async error happens before completion, not after. That's inconsistent. Error handling is hard enough without race conditions.
  • should the first sync error not be passed to the onError callback? If we let it escape, then it shouldn't also be unhandled. But what if the main computation does have an error, should we then go back and pass the first unhandled error to onError anyway? And why the first error.

Seems more consistent to not use an existing error, but instead replace the successful completion with a StateError("Async failure during computation"), to signal that the computation was not successful, but leave the actual unhandled errors to all be handled by onError.
It's still a race condition, but at least it's symmetrical in the unhandled errors.

from async.

natebosch avatar natebosch commented on July 16, 2024

I think that'll be mostly non-breaking for reasonable uses.

We should go through the breaking change process and double check this assumption. I think the typical patterns to work around current behavior should be OK since they typically check completer.isCompleted and should be able to ignore the extra new error completion, or at least not cause a new exception when it occurs.

I can imagine that some implementations may double-log an error if it starts to also surface as an error future in the calling zone.

Edit: If we only surface this error through the returned Future and stop calling the onError callback with it - as would match the design I proposed here - I do think this could cause problems. It's very likely that existing implementations expect that the onError callback would be called, and likely don't have an equivalent error handler on the Future.

makes it replace a successful completion with an error if there has been any unhandled async error before completion.

I think current workarounds are more likely to surface the error immediately and not wait for the successful completion.

from async.

lrhn avatar lrhn commented on July 16, 2024

I checked my assumptions, and it won't work. Turns out the runZoneGuarded returns null in case of a synchronous error. There is no provision for returning Future.value(null) for an async error, and it's probably not even possible to type it. And it's not possible to return null synchronously for an async error.

While we can keep doing that, and make an async error future smuggle the error out (maybe), it's not consistent.
On the other hand, a future which never completes is probably worse than an inconsistent async error.
So I'll try to find a way to make a returned error future be intercepted, letting onError handle the error, and return a future which throws a fixed error from the outside zone, like the StateError above.
Basically change the non-completing future to a future which completes with a marker error, to say that it couldn't complete with the real error.

But it would also change the behavior of never needing to handle errors from a returned future.
(How about we just deprecate runZonedGuarded and create a better API, one which does emit errors in the result, and only cares about catching uncaught errors, and maybe another one which catches all errors and returns Future<void> when it's done.)

I'm not very keen on eagerly completing the returned future with an unhandled async error. For two reasons.

  1. It's arbitrary. We take an error from one location and surfaces it in another location. It doesn't stop the computation, and it assumes that an async error in some spun-off computation means that the main computation can't complete. Maybe it can, and that's precisely why the user added onError to gently handle the things that fail, until something succeeds. (Not great design, they should catch errors instead, but it's not an invalid use.)
  2. More importantly, it can't really work. The runZonedGuarded isn't required to return a future to begin with. It must return, synchronously, a value of type R?, where we have no idea what R is. We can check if it actually does return a future, but it's incredibly hard to do anything with that future and preserve its type, when we don't know what the type is. (We can use catchError and whenComplete, since those are guaranteed to retain the original type, but we can't change the element type, and we can't create a completer of the same type, which means it'll be very hard to actually smuggle an error past the error zone boundary.)

I almost had a way of smuggling an error past the zone, by doing .asStream() inside the zone, and .first outside, but that still won't work if someone uses a subtype of Future:

class Banana implements Future<bool> { Future<bool> _banana; ... }
...
  Banana createBanana() => ...
  var b = runZoneGuarded(createBanana, (e, s) => ...);

Here the type parameter, and type of of b, is Banana. There is absolutely no way one can intercept a Banana, recognize that it's a future, extract the error, smuggle it out, and put it back into a Banana.

We might be able to do the hack if the type parameter is Future<X> or FutureOr<X>, but we may still end up changing the kind of Future being returned (which could, for example, be a SyncFuture).

So, all in all, runZonedGuarded is probably unsalvageable. Let's scrap it and create a new and better one instead :)
Like you suggested, just without "smuggle" in the name.

from async.

lrhn avatar lrhn commented on July 16, 2024

A possible alternative could be:

import "dart:async";

/// Runs [action] in a new [Zone] where [onError] handles uncaught errors.
///
/// Returns the result of [action]. If that result is a [Future], it is
/// copied into a future belonging to the surrounding zone, so that
/// an error can be handled.
Future<R> catchUnhandledAsyncErrors<R>(
  FutureOr<R> Function() action, {
  required void Function(Object, StackTrace) onError,
  ZoneSpecification? zoneSpecification,
}) {
  void handleError(Zone self, ZoneDelegate parent, Zone zone, Object error,
      StackTrace stack) {
    onError(error, stack);
  }

  var spec = zoneSpecification == null
      ? ZoneSpecification(handleUncaughtError: handleError)
      : ZoneSpecification.from(zoneSpecification,
          handleUncaughtError: handleError);

  var outer = Zone.current;
  return runZoned<Future<R>>(zoneSpecification: spec, () {
    FutureOr<R> result;
    try {
      result = action();
    } catch (e, s) {
      return outer.run(() => Future<R>.error(e, s));
    }
    if (result is Future<R>) {
      var c = outer.run(() => Completer<R>.sync());
      result.then(c.complete, onError: c.completeError);
      return c.future;
    } else {
      return Future<R>.value(result);
    }
  });
}


void main() {
  FutureOr<bool> banana() async {
    Future.error("Plantain");
    throw "Banana";
  }
  var f = catchUnhandledAsyncErrors(banana, onError: (e, s) {
    print("Uncaught: $e");
  });
  f.catchError((e, s) {
    print("Caught: $e");
    return true;
  });
}

from async.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.