Improvement on future: add stack trace to future failures.

Improvement on future: add stack trace to future failures.

Committed: https://code.google.com/p/dart/source/detail?r=13849

[Siggi Cherem (dart-lang), 2012-10-19 22:58:19]

Basically Instead of seeing this:
Unhandled exception:
type 'String' is not a subtype of type 'Path' of 'userMainImport'.
#0      FutureImpl._complete (bootstrap_impl:609:11)
#1      FutureImpl._complete (bootstrap_impl:613:5)
#2      FutureImpl._setException (bootstrap_impl:638:14)
#3      CompleterImpl.completeException (bootstrap_impl:713:30)
#4      FutureImpl._forwardException.<anonymous closure> (bootstrap_impl:697:34)
#5      FutureImpl._complete (bootstrap_impl:597:22)
#6      FutureImpl._complete (bootstrap_impl:613:5)
#7      FutureImpl._setException (bootstrap_impl:638:14)
#8      CompleterImpl.completeException (bootstrap_impl:713:30)
#9      FutureImpl.transform.<anonymous closure> (bootstrap_impl:649:36)
#10     FutureImpl._complete (bootstrap_impl:605:19)
#11     FutureImpl._complete (bootstrap_impl:613:5)
#12     FutureImpl._setValue (bootstrap_impl:627:14)
#13     CompleterImpl.complete (bootstrap_impl:710:26)
...

The new output is this:

Unhandled exception:
type 'String' is not a subtype of type 'Path' of 'userMainImport'.
  #0      bootstrapCode (package:web_components/src/codegen.dart:106:27)
  #1      Compiler._emitMainHtml
(package:web_components/src/compiler.dart:195:32)
  #2      Compiler._emit.<anonymous closure>
(package:web_components/src/compiler.dart:164:26)
  #3      time (package:web_components/src/utils.dart:34:24)
  #4      Compiler._emit (package:web_components/src/compiler.dart:156:11)
  #5      Compiler.run.<anonymous closure>
(package:web_components/src/compiler.dart:67:12)
  #6      FutureImpl.transform.<anonymous closure> (bootstrap_impl:646:37)
  
#0      FutureImpl._complete (bootstrap_impl:609:11)
#1      FutureImpl._complete (bootstrap_impl:613:5)
#2      FutureImpl._setException (bootstrap_impl:638:14)
#3      CompleterImpl.completeException (bootstrap_impl:713:30)
#4      FutureImpl._forwardException.<anonymous closure> (bootstrap_impl:697:34)
#5      FutureImpl._complete (bootstrap_impl:597:22)
#6      FutureImpl._complete (bootstrap_impl:613:5)
#7      FutureImpl._setException (bootstrap_impl:638:14)
#8      CompleterImpl.completeException (bootstrap_impl:713:30)
#9      FutureImpl.transform.<anonymous closure> (bootstrap_impl:649:36)
#10     FutureImpl._complete (bootstrap_impl:605:19)
#11     FutureImpl._complete (bootstrap_impl:613:5)
#12     FutureImpl._setValue (bootstrap_impl:627:14)
#13     CompleterImpl.complete (bootstrap_impl:710:26)
...

[Jennifer Messerly, 2012-10-19 23:54:15]

fwiw, lgtm

https://codereview.chromium.org/11225017/diff/2001/lib/core/future.dart
File lib/core/future.dart (right):

https://codereview.chromium.org/11225017/diff/2001/lib/core/future.dart#newco...
lib/core/future.dart:208: FutureUnhandledException(this.source,
this.stackTrace);
nit: newline after this

https://codereview.chromium.org/11225017/diff/2001/lib/core/future.dart#newco...
lib/core/future.dart:209: String toString() =>
"$source\n$stackTrace".replaceAll('\n','\n  ');
include more context here? maybe something like:

FutureUnhandledException: exception while executing Future:
  ${source.replaceAll('\n','\n  ')}
original stack trace:
  ${stackTrace.replaceAll('\n','\n  ')}

[Siggi Cherem (dart-lang), 2012-10-20 00:22:51]

thanks, done. Submitting!

[Bob Nystrom, 2012-10-22 16:57:49]

https://codereview.chromium.org/11225017/diff/7001/tests/corelib/future_test....
File tests/corelib/future_test.dart (right):

https://codereview.chromium.org/11225017/diff/7001/tests/corelib/future_test....
tests/corelib/future_test.dart:77: Expect.throws(() => future.value, (e) =>
e.source == exception);
I'm all for better error messages in future, but I'm a little worried about
this.

Does this mean that code that handles an exception from a future will always
receive the wrapped error and not the original one? Is this documented
somewhere?

[Siggi Cherem (dart-lang), 2012-10-22 17:23:54]

https://codereview.chromium.org/11225017/diff/7001/tests/corelib/future_test....
File tests/corelib/future_test.dart (right):

https://codereview.chromium.org/11225017/diff/7001/tests/corelib/future_test....
tests/corelib/future_test.dart:77: Expect.throws(() => future.value, (e) =>
e.source == exception);
On 2012/10/22 16:57:49, Bob Nystrom wrote:
> I'm all for better error messages in future, but I'm a little worried about
> this.
> 
> Does this mean that code that handles an exception from a future will always
> receive the wrapped error and not the original one? Is this documented
> somewhere?

The wrapping will only happen when the exception is thrown in the open. The
exception manipulation functions, like 'handleException' or
'transformException', will continue to work the same as before.

[Jennifer Messerly, 2012-10-22 20:51:25]

https://codereview.chromium.org/11225017/diff/7001/tests/corelib/future_test....
File tests/corelib/future_test.dart (right):

https://codereview.chromium.org/11225017/diff/7001/tests/corelib/future_test....
tests/corelib/future_test.dart:77: Expect.throws(() => future.value, (e) =>
e.source == exception);
On 2012/10/22 17:23:54, Siggi Cherem (dart-lang) wrote:
> On 2012/10/22 16:57:49, Bob Nystrom wrote:
> > I'm all for better error messages in future, but I'm a little worried about
> > this.
> > 
> > Does this mean that code that handles an exception from a future will always
> > receive the wrapped error and not the original one? Is this documented
> > somewhere?
> 
> The wrapping will only happen when the exception is thrown in the open. The
> exception manipulation functions, like 'handleException' or
> 'transformException', will continue to work the same as before.

Yeah, I think the idea is: if you handle the exception in your future, you get
the original exception object.

If you *do not* handle the exception, you get it at completion time. So wrapping
it is okay, because the code that completes the future wouldn't be able to
handle the exception anyway. The future-completion code is trying to send the
exception to the receiver of the future, but nobody is listening for exceptions.
So wrapping it and throwing it all the way up the call stack to the top level
handler makes sense.

At least, that's how I justified it in my mind when I read the CL the first time
:)

[Bob Nystrom, 2012-10-22 22:34:18]

On 2012/10/22 20:51:25, John Messerly wrote:
>
https://codereview.chromium.org/11225017/diff/7001/tests/corelib/future_test....
> File tests/corelib/future_test.dart (right):
> 
>
https://codereview.chromium.org/11225017/diff/7001/tests/corelib/future_test....
> tests/corelib/future_test.dart:77: Expect.throws(() => future.value, (e) =>
> e.source == exception);
> On 2012/10/22 17:23:54, Siggi Cherem (dart-lang) wrote:
> > On 2012/10/22 16:57:49, Bob Nystrom wrote:
> > > I'm all for better error messages in future, but I'm a little worried
about
> > > this.
> > > 
> > > Does this mean that code that handles an exception from a future will
always
> > > receive the wrapped error and not the original one? Is this documented
> > > somewhere?
> > 
> > The wrapping will only happen when the exception is thrown in the open. The
> > exception manipulation functions, like 'handleException' or
> > 'transformException', will continue to work the same as before.
> 
> Yeah, I think the idea is: if you handle the exception in your future, you get
> the original exception object.
> 
> If you *do not* handle the exception, you get it at completion time. So
wrapping
> it is okay, because the code that completes the future wouldn't be able to
> handle the exception anyway. The future-completion code is trying to send the
> exception to the receiver of the future, but nobody is listening for
exceptions.
> So wrapping it and throwing it all the way up the call stack to the top level
> handler makes sense.
> 
> At least, that's how I justified it in my mind when I read the CL the first
time
> :)

Ah, now I get it. So you only get the wrapped exception when then() does a real
throw because no exception handlers are attached?

In that case, this sounds good. Though that fact should be documented.