Issue 926753002: Improve DartLoader error handling.

Issue 926753002: Improve DartLoader error handling. (Closed)

Created:
5 years, 10 months ago by eseidel

Modified:
5 years, 10 months ago

Reviewers:
abarth-chromium

CC:
Aaron Boodman, ben+mojo_chromium.org, darin (slow to review), esprehn, mojo-reviews_chromium.org, ojan, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org

Base URL:
git@github.com:domokit/mojo.git@master

Target Ref:
refs/heads/master

Project:
mojo

Visibility:
Public.

More Reviews

Description

Improve DartLoader error handling. Before this if a dart import 404s, we crash. This makes some minor improvements in our logging for better diagnosis of future errors. R=abarth@chromium.org BUG= Committed: https://chromium.googlesource.com/external/mojo/+/40a6b795cf600af3c74cea3c2b22bb95c6810705

Patch Set 2 : Remove saw_error #

Total comments: 1

Created: 5 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+30 lines, -3 lines)			Patch
M	mojo/common/data_pipe_drainer.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	sky/engine/core/script/dart_loader.h	View	1	1 chunk	+1 line, -0 lines	0 comments	Download
M	sky/engine/core/script/dart_loader.cc	View	1	3 chunks	+19 lines, -2 lines	1 comment	Download
M	sky/engine/tonic/dart_error.cc	View		1 chunk	+9 lines, -0 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

eseidel

I should probably add a 404 test, but I don't think we correctly route console ...

5 years, 10 months ago (2015-02-13 23:47:10 UTC) #1

abarth-chromium

https://codereview.chromium.org/926753002/diff/1/sky/engine/core/script/dart_loader.cc File sky/engine/core/script/dart_loader.cc (right): https://codereview.chromium.org/926753002/diff/1/sky/engine/core/script/dart_loader.cc#newcode105 sky/engine/core/script/dart_loader.cc:105: if (!loader_->saw_error()) Why do we only do this once ...

5 years, 10 months ago (2015-02-13 23:50:29 UTC) #2

abarth-chromium

LGTM https://codereview.chromium.org/926753002/diff/20001/sky/engine/core/script/dart_loader.cc File sky/engine/core/script/dart_loader.cc (right): https://codereview.chromium.org/926753002/diff/20001/sky/engine/core/script/dart_loader.cc#newcode182 sky/engine/core/script/dart_loader.cc:182: dependency_catcher_(nullptr) { git cl format will probably undo ...

5 years, 10 months ago (2015-02-14 00:28:06 UTC) #4

abarth-chromium

Committed patchset #2 (id:20001) manually as 40a6b795cf600af3c74cea3c2b22bb95c6810705 (presubmit successful).

5 years, 10 months ago (2015-02-15 04:35:30 UTC) #5

abarth-chromium

5 years, 10 months ago (2015-02-15 04:36:22 UTC) #6

Message was sent while issue was closed.

I landed this for Eric because Raf was hitting this issue.

Expand Messages | Collapse Messages