Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception handling in futures is clumsy. #3047

Closed
DartBot opened this issue May 14, 2012 · 5 comments
Closed

Exception handling in futures is clumsy. #3047

DartBot opened this issue May 14, 2012 · 5 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented May 14, 2012

This issue was originally filed by sammcca...@google.com


* Requires me to register two handlers

What about

someAsyncMethod().then((result) {
  try {
    print(result.value);
  } catch (SomeException e) {
    print("Oh no, error!");
  }
});

'result' could be the original Future object, or some Completion<T> object (which would probably play nicer with a unified event/future async model)

@sethladd
Copy link
Contributor

cc @sigmundch.
Removed Type-Defect label.
Added Type-Enhancement, Area-Library, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Jun 1, 2012

This comment was originally written by sammcca...@google.com


Additional shortcomings from more experience:
The hidden-statefulness of Futures makes composability really bad. (In this case the hidden state is: whether there is an error handler already attached which will ultimately swallow an error that occurs).
For example, given an arbitrary future, there is no way to trigger a callback whenever it completes, and no way to know whether an error handler you attach is dead.

One workaround would be to add a Future.finally(void callback()) that would execute after all .then()s and .handleException()s regardless of the completion outcome.
That wouldn't break existing code, but would allow users to ignore the problematic behaviour by using:

    future.handleException((e) => true);
    future.finally((){ ... });

Would you accept a patch for this?

@DartBot
Copy link
Author

DartBot commented Jun 7, 2012

This comment was originally written by sammcca...@google.com


Sorry, I forgot the BUG= line in the code review, but this is fixed in https://code.google.com/p/dart/source/detail?r=8303

@whesse
Copy link
Member

whesse commented Sep 11, 2012

I disagree with the way that any exceptions in the onComplete handler are silently ignored. I think there is a principle that no exceptions are silently swallowed anywhere in Dart. I had a hard time finding and debugging errors in my onComplete handlers because of this.

This is now filed as part of bug 4127.

@DartBot DartBot added Type-Enhancement area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. labels Sep 11, 2012
@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug and removed triaged labels Feb 29, 2016
@lrhn
Copy link
Member

lrhn commented Aug 11, 2017

No plans to change the Future design in this way.
Each handler on a future is notified individually, there is no "after all handlers are notified" time that you can use. You can add new handlers to a future after it has completed.

The onComplete will not swallow errors unless a new error is introduced. In that way it works the same as a try/finally where the finally-block throws.

You can now either use async functions to use try/catch on awaited futures, or you can use Result.capture from package:async to capture the result of a future into a single object that can hold either a value or an exception.

@lrhn lrhn closed this as completed Aug 11, 2017
@lrhn lrhn added the closed-not-planned Closed as we don't intend to take action on the reported issue label Aug 11, 2017
copybara-service bot pushed a commit that referenced this issue Aug 5, 2022
…2 revisions)

https://dart.googlesource.com/dartdoc/+log/bd57c0e7b756..f419695f57c5

2022-08-05 srawlins@google.com Fork built-in features into _BuiltInFeature (#3105)
2022-08-04 106621169+klr981@users.noreply.github.com Adopt badge design for all features #3047 (#3101)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/dart-doc-dart-sdk
Please CC dart-ecosystem-gardener@grotations.appspotmail.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Dart Documentation Generator: https://github.com/dart-lang/dartdoc/issues
To file a bug in Dart SDK: https://github.com/dart-lang/sdk/issues

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Tbr: dart-ecosystem-gardener@grotations.appspotmail.com
Change-Id: I6b7680fa427fac056756e69700c005ae69bedc79
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/253721
Reviewed-by: Devon Carew <devoncarew@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants