repl: attach location info to syntax errors #4013

cjihrig · 2015-11-25T01:04:40Z

Currently, when a file with a syntax error is imported in the REPL, no information is provided on the error's location. This commit adds the error's location to the stack trace.

Refs #3784

thefourtheye · 2015-11-25T03:10:59Z

test/parallel/test-repl-syntax-error-stack.js

+ArrayStream.prototype.readable = true;
+ArrayStream.prototype.writable = true;
+ArrayStream.prototype.resume = function() {};
+ArrayStream.prototype.write = function(output) {


We can use common.mustCall here to make sure that this is called.

bnoordhuis · 2015-11-25T11:02:53Z

I agree with the comments from @targos and @thefourtheye but LGTM apart from that.

cjihrig · 2015-11-25T13:22:52Z

@thefourtheye @targos @bnoordhuis responded to comments.

cjihrig · 2015-11-25T16:07:07Z

Starting a CI run: https://ci.nodejs.org/job/node-test-pull-request/847/

Please let me know if I haven't adequately addressed your comments.

UPDATE: The test appears to have some issues on Windows.

cjihrig · 2015-11-25T17:55:57Z

New CI because Windows backslashes + REPL: https://ci.nodejs.org/job/node-test-pull-request/848/

This commit adds the decorateErrorStack() method. This function uses the internal util's getHiddenValue() method to extract arrow messages from error objects and attach them to the error's stack trace. PR-URL: nodejs#4013 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Currently, when a file with a syntax error is imported in the REPL, no information is provided on the error's location. This commit adds the error's location to the stack trace. Refs: nodejs#2762 Refs: nodejs#3411 Refs: nodejs#3784 PR-URL: nodejs#4013 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

bnoordhuis · 2015-11-25T20:07:58Z

I'm going to look into moving .decorateErrorStack to internal/util.js. I believe the reason it doesn't work right now is because of a REPL-specific hack in the module loader.

EDIT: #4026

jasnell · 2015-11-30T22:17:58Z

@cjihrig : should this go into LTS also?

cjihrig · 2015-11-30T22:31:21Z

It can go into LTS, but @bnoordhuis has opened #4026 to make the decorateErrorStack() method (introduced by this PR) internal. So, they need to be a package deal. The commits in this PR also depend on #3988.

bnoordhuis · 2015-11-30T23:05:05Z

I just landed #4026, FWIW.

This commit adds the decorateErrorStack() method. This function uses the internal util's getHiddenValue() method to extract arrow messages from error objects and attach them to the error's stack trace. PR-URL: #4013 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Currently, when a file with a syntax error is imported in the REPL, no information is provided on the error's location. This commit adds the error's location to the stack trace. Refs: #2762 Refs: #3411 Refs: #3784 PR-URL: #4013 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins · 2015-12-15T01:08:50Z

This PR will also require #4215 to land on LTS in order to not break the repl.

Will wait for it to land and live in a release for a bit before landing these commits in LTS

This commit adds the decorateErrorStack() method. This function uses the internal util's getHiddenValue() method to extract arrow messages from error objects and attach them to the error's stack trace. PR-URL: nodejs#4013 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Currently, when a file with a syntax error is imported in the REPL, no information is provided on the error's location. This commit adds the error's location to the stack trace. Refs: nodejs#2762 Refs: nodejs#3411 Refs: nodejs#3784 PR-URL: nodejs#4013 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This commit adds the decorateErrorStack() method. This function uses the internal util's getHiddenValue() method to extract arrow messages from error objects and attach them to the error's stack trace. PR-URL: #4013 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Currently, when a file with a syntax error is imported in the REPL, no information is provided on the error's location. This commit adds the error's location to the stack trace. Refs: #2762 Refs: #3411 Refs: #3784 PR-URL: #4013 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Notable changes: * assert - accommodate ES6 classes that extend Error (Rich Trott) #4166 * build - add "--partly-static" build options (Super Zheng) #4152 * deps - backport 066747e from upstream V8 (Ali Ijaz Sheikh) #4655 - backport 200315c from V8 upstream (Vladimir Kurchatkin) #4128 - upgrade libuv to 1.8.0 (Saúl Ibarra Corretgé) * docs - various updates landed in 70 different commits! * repl - attach location info to syntax errors (cjihrig) #4013 - display error message when loading directory (Prince J Wesley) #4170 * tests - various updates landed in over 50 commits * util - allow lookup of hidden values (cjihrig) #3988

Notable changes: * assert - accommodate ES6 classes that extend Error (Rich Trott) #4166 * build - add "--partly-static" build options (Super Zheng) #4152 * deps - backport 066747e from upstream V8 (Ali Ijaz Sheikh) #4655 - backport 200315c from V8 upstream (Vladimir Kurchatkin) #4128 - upgrade libuv to 1.8.0 (Saúl Ibarra Corretgé) * docs - various updates landed in 70 different commits! * repl - attach location info to syntax errors (cjihrig) #4013 - display error message when loading directory (Prince J Wesley) #4170 * tests - various updates landed in over 50 commits * util - allow lookup of hidden values (cjihrig) #3988 PR-URL: #4768

This commit adds the decorateErrorStack() method. This function uses the internal util's getHiddenValue() method to extract arrow messages from error objects and attach them to the error's stack trace. PR-URL: nodejs#4013 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Currently, when a file with a syntax error is imported in the REPL, no information is provided on the error's location. This commit adds the error's location to the stack trace. Refs: nodejs#2762 Refs: nodejs#3411 Refs: nodejs#3784 PR-URL: nodejs#4013 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Notable changes: * assert - accommodate ES6 classes that extend Error (Rich Trott) nodejs#4166 * build - add "--partly-static" build options (Super Zheng) nodejs#4152 * deps - backport 066747e from upstream V8 (Ali Ijaz Sheikh) nodejs#4655 - backport 200315c from V8 upstream (Vladimir Kurchatkin) nodejs#4128 - upgrade libuv to 1.8.0 (Saúl Ibarra Corretgé) * docs - various updates landed in 70 different commits! * repl - attach location info to syntax errors (cjihrig) nodejs#4013 - display error message when loading directory (Prince J Wesley) nodejs#4170 * tests - various updates landed in over 50 commits * util - allow lookup of hidden values (cjihrig) nodejs#3988 PR-URL: nodejs#4768

mscdex added the repl Issues and PRs related to the REPL subsystem. label Nov 25, 2015

thefourtheye reviewed Nov 25, 2015
View reviewed changes

cjihrig force-pushed the stacks branch from c01c1b7 to e04a4a7 Compare November 25, 2015 17:53

cjihrig added 2 commits November 25, 2015 13:28

cjihrig force-pushed the stacks branch from e04a4a7 to 459b106 Compare November 25, 2015 18:43

cjihrig merged commit 459b106 into nodejs:master Nov 25, 2015

cjihrig deleted the stacks branch November 25, 2015 18:47

Fishrock123 mentioned this pull request Nov 26, 2015

deps: upgrade npm to 3.5.0 #4032

Closed

cjihrig mentioned this pull request Nov 30, 2015

util,src: allow lookup of hidden values #3988

Merged

MylesBorins mentioned this pull request Dec 1, 2015

test: move ArrayStream to common #4027

Merged

MylesBorins added the lts-watch-v4.x label Dec 1, 2015

This was referenced Dec 8, 2015

Release proposal: v5.2.0 #4180

Closed

Release proposal: v5.2.0 (Stable) #4181

Merged

rvagg mentioned this pull request Dec 17, 2015

Release 4.2.4 Planning #4321

Closed

MylesBorins mentioned this pull request Dec 30, 2015

Repl Changes to LTS blocked #4472

Closed

MylesBorins removed the lts-watch-v4.x label Jan 7, 2016

MylesBorins mentioned this pull request Jan 19, 2016

V4.2.5 proposal #4768

Merged

strrchr mentioned this pull request Feb 18, 2016

Need SyntaxError location when require() failed in uncaughtException handler #5297

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

repl: attach location info to syntax errors #4013

repl: attach location info to syntax errors #4013

cjihrig commented Nov 25, 2015

thefourtheye Nov 25, 2015

bnoordhuis commented Nov 25, 2015

cjihrig commented Nov 25, 2015

cjihrig commented Nov 25, 2015

cjihrig commented Nov 25, 2015

bnoordhuis commented Nov 25, 2015

jasnell commented Nov 30, 2015

cjihrig commented Nov 30, 2015

bnoordhuis commented Nov 30, 2015

MylesBorins commented Dec 15, 2015

repl: attach location info to syntax errors #4013

repl: attach location info to syntax errors #4013

Conversation

cjihrig commented Nov 25, 2015

thefourtheye Nov 25, 2015

Choose a reason for hiding this comment

bnoordhuis commented Nov 25, 2015

cjihrig commented Nov 25, 2015

cjihrig commented Nov 25, 2015

cjihrig commented Nov 25, 2015

bnoordhuis commented Nov 25, 2015

jasnell commented Nov 30, 2015

cjihrig commented Nov 30, 2015

bnoordhuis commented Nov 30, 2015

MylesBorins commented Dec 15, 2015