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

repl: attach location info to syntax errors #4013

Merged
merged 2 commits into from
Nov 25, 2015
Merged

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Nov 25, 2015

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

@mscdex mscdex added the repl Issues and PRs related to the REPL subsystem. label Nov 25, 2015
ArrayStream.prototype.readable = true;
ArrayStream.prototype.writable = true;
ArrayStream.prototype.resume = function() {};
ArrayStream.prototype.write = function(output) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@bnoordhuis
Copy link
Member

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

@cjihrig
Copy link
Contributor Author

cjihrig commented Nov 25, 2015

@thefourtheye @targos @bnoordhuis responded to comments.

@cjihrig
Copy link
Contributor Author

cjihrig commented Nov 25, 2015

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
Copy link
Contributor Author

cjihrig commented Nov 25, 2015

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>
@cjihrig cjihrig merged commit 459b106 into nodejs:master Nov 25, 2015
@cjihrig cjihrig deleted the stacks branch November 25, 2015 18:47
@bnoordhuis
Copy link
Member

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
Copy link
Member

jasnell commented Nov 30, 2015

@cjihrig : should this go into LTS also?

@cjihrig
Copy link
Contributor Author

cjihrig commented Nov 30, 2015

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
Copy link
Member

I just landed #4026, FWIW.

cjihrig added a commit that referenced this pull request Dec 5, 2015
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>
cjihrig added a commit that referenced this pull request Dec 5, 2015
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
Copy link
Contributor

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

@rvagg rvagg mentioned this pull request Dec 17, 2015
cjihrig added a commit to cjihrig/node that referenced this pull request Jan 7, 2016
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>
cjihrig added a commit to cjihrig/node that referenced this pull request Jan 7, 2016
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>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
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>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
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 MylesBorins mentioned this pull request Jan 19, 2016
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
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
MylesBorins pushed a commit that referenced this pull request Jan 20, 2016
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
MylesBorins pushed a commit that referenced this pull request Jan 20, 2016
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
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
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>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
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>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants