Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(207)

Issue 945783002: Dart2js async-await. Propagate stacktraces from futures. (Closed)

Created:
5 years, 10 months ago by sigurdm
Modified:
5 years, 10 months ago
Reviewers:
floitsch
CC:
reviews_dartlang.org
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Dart2js async-await. Propagate stacktraces from futures. BUG= dartbug.com/22324 R=floitsch@google.com Committed: https://code.google.com/p/dart/source/detail?r=43951

Patch Set 1 #

Patch Set 2 : Add test #

Patch Set 3 : Fix status file #

Total comments: 6

Patch Set 4 : Address review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+147 lines, -6 lines) Patch
M sdk/lib/_internal/compiler/js_lib/js_helper.dart View 1 2 3 5 chunks +31 lines, -5 lines 0 comments Download
A tests/compiler/dart2js_extra/async_stacktrace_test.dart View 1 2 3 1 chunk +116 lines, -0 lines 0 comments Download
M tests/language/language_dart2js.status View 1 2 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 6 (2 generated)
sigurdm
5 years, 10 months ago (2015-02-20 09:22:50 UTC) #2
floitsch
LGTM. https://codereview.chromium.org/945783002/diff/40001/tests/compiler/dart2js_extra/async_stacktrace_test.dart File tests/compiler/dart2js_extra/async_stacktrace_test.dart (right): https://codereview.chromium.org/945783002/diff/40001/tests/compiler/dart2js_extra/async_stacktrace_test.dart#newcode22 tests/compiler/dart2js_extra/async_stacktrace_test.dart:22: Expect.equals(savedStackTrace.toString(), st.toString()); Make sure this expect is reached. ...
5 years, 10 months ago (2015-02-20 12:36:05 UTC) #3
sigurdm
Committed patchset #4 (id:60001) manually as 43951 (presubmit successful).
5 years, 10 months ago (2015-02-23 12:41:04 UTC) #5
sigurdm
5 years, 10 months ago (2015-02-23 12:41:18 UTC) #6
Message was sent while issue was closed.
https://codereview.chromium.org/945783002/diff/40001/tests/compiler/dart2js_e...
File tests/compiler/dart2js_extra/async_stacktrace_test.dart (right):

https://codereview.chromium.org/945783002/diff/40001/tests/compiler/dart2js_e...
tests/compiler/dart2js_extra/async_stacktrace_test.dart:22:
Expect.equals(savedStackTrace.toString(), st.toString());
On 2015/02/20 12:36:05, floitsch wrote:
> Make sure this expect is reached. Same for the others.

Done.

https://codereview.chromium.org/945783002/diff/40001/tests/compiler/dart2js_e...
tests/compiler/dart2js_extra/async_stacktrace_test.dart:70: ;
On 2015/02/20 12:36:05, floitsch wrote:
> move ";" to previous line.
> Same for test3.

Done.

https://codereview.chromium.org/945783002/diff/40001/tests/language/asyncstar...
File tests/language/asyncstar_throw_in_catch_test.dart (right):

https://codereview.chromium.org/945783002/diff/40001/tests/language/asyncstar...
tests/language/asyncstar_throw_in_catch_test.dart:96: await runTest(foo1,
"abcdYfC", null, true);
On 2015/02/20 12:36:05, floitsch wrote:
> This looks a change from a different CL.

Yes it is - I must have made a mistake during upload. But it is from
https://codereview.chromium.org/947433002/  And already comitted.

Powered by Google App Engine
This is Rietveld 408576698