repl: better handling of recoverable errors #18915

princejwesley · 2018-02-21T20:23:00Z

Better handling of recoverable errors in REPL module

Below syntax errors are handled without force .break/clear

Unexpected Token (prefix errors)
missing ) after argument list

In the multiline expression, recoverable errors are truly
recoverable, otherwise syntax error will be thrown.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

repl

Leko

LGTM

Leko · 2018-02-24T16:26:21Z

CI: https://ci.nodejs.org/job/node-test-pull-request/13359/

Leko · 2018-02-25T12:08:44Z

CI failed but I think those errors are not related to this PR.

not ok 712 parallel/test-http-pause
  ---
  duration_ms: 0.213
  severity: fail
  stack: |-
  ...

https://ci.nodejs.org/job/node-test-commit-linux/nodes=debian8-64/16655/console

not ok 2002 sequential/test-fs-readfile-tostring-fail
  ---
  duration_ms: 8.50
  severity: crashed
  stack: |-
    oh no!
    exit code: CRASHED (Signal: 9)
  ...

https://ci.nodejs.org/job/node-test-commit-linux/nodes=centos7-64/16655/console

Leko · 2018-02-26T16:48:10Z

CI again: https://ci.nodejs.org/job/node-test-pull-request/13375/

0joshuaolson1 · 2018-02-27T00:05:40Z

Is this the right place to add other syntax errors that the repl doesn't handle correctly? I can't find an issue for it.

BridgeAR

LGTM. Just a nit and a question.

BridgeAR · 2018-03-02T01:00:25Z

lib/repl.js

+      const pos = frames.findIndex((f) => f.match(/^\s*\^+$/));
+      if (pos > 0 && frames[pos - 1].length === frames[pos].length)
+        return true;
+      else return false;


Nit: please remove the else. This is a style that is normally not used here.

rebasing and amending to same commit

BridgeAR · 2018-03-02T01:01:55Z

test/parallel/test-repl.js

@@ -163,13 +163,23 @@ const errorTests = [
    send: '.break',
    expect: ''
  },
-  // Template expressions can cross lines
+  // Template expressions
  {
    send: '`io.js ${"1.0"',


I guess recovering from e.g.

'`abc ${ test'

is not possible anymore?

Yes, only some forms are not possible. For instance, below one is possible.

`abc ${ test}`

Its mostly consistent with chrome console behaviour except for couple of cases.

we handle 'missing ) after argument list' error better than chrome console.

we allow repl commands in multiline mode which should otherwise be treated as properties/functions.
princejwesley@c1796b8 (wip)

> (function() { ... x = { help: () => 'help received' }; ... return x ... .help (); <-- no repl command parsing in multiline mode .break Sometimes you get stuck, this gets you out .clear Alias for .break .editor Enter editor mode .exit Exit the repl .help Print this help message .load Load JS from a file into the REPL session .save Save all evaluated commands in this REPL session to a file ...

(I'll give PR over the weekend)

BridgeAR · 2018-03-02T01:03:38Z

@0joshuaolson1 please open a new issue in case you think the repl should recover from errors that you ran into.

Below syntax errors are handled without force .break/clear - Unexpected Token (prefix errors) - missing ) after argument list In the multiline expression, recoverable errors are truly recoverable, otherwise syntax error will be thrown.

BridgeAR · 2018-03-06T19:47:05Z

CI https://ci.nodejs.org/job/node-test-pull-request/13543/

BridgeAR · 2018-03-11T02:33:47Z

Landed in ebfa8b1 🎉

Below syntax errors are handled without force .break/clear - Unexpected Token (prefix errors) - missing ) after argument list In the multiline expression, recoverable errors are truly recoverable, otherwise syntax error will be thrown. PR-URL: #18915 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

Below syntax errors are handled without force .break/clear - Unexpected Token (prefix errors) - missing ) after argument list In the multiline expression, recoverable errors are truly recoverable, otherwise syntax error will be thrown. PR-URL: nodejs#18915 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

Below syntax errors are handled without force .break/clear - Unexpected Token (prefix errors) - missing ) after argument list In the multiline expression, recoverable errors are truly recoverable, otherwise syntax error will be thrown. Backport-PR-URL: #22380 PR-URL: #18915 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

nodejs-github-bot added the repl Issues and PRs related to the REPL subsystem. label Feb 21, 2018

addaleax approved these changes Feb 21, 2018

View reviewed changes

Leko approved these changes Feb 24, 2018

View reviewed changes

daynin approved these changes Feb 24, 2018

View reviewed changes

jasnell approved these changes Feb 26, 2018

View reviewed changes

0joshuaolson1 mentioned this pull request Feb 27, 2018

repl: do not consider ... as a REPL command #14467

Closed

3 tasks

BridgeAR approved these changes Mar 2, 2018

View reviewed changes

BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 2, 2018

repl: better handling of recoverable errors

b9f6453

Below syntax errors are handled without force .break/clear - Unexpected Token (prefix errors) - missing ) after argument list In the multiline expression, recoverable errors are truly recoverable, otherwise syntax error will be thrown.

princejwesley force-pushed the repl.cont branch from 31c97b4 to b9f6453 Compare March 2, 2018 13:32

BridgeAR closed this Mar 11, 2018

targos mentioned this pull request Mar 18, 2018

v9.9.0 proposal #19428

Merged

MylesBorins mentioned this pull request Sep 6, 2018

v8.12.0 proposal #21593

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

repl: better handling of recoverable errors #18915

repl: better handling of recoverable errors #18915

princejwesley commented Feb 21, 2018

Leko left a comment

Leko commented Feb 24, 2018

Leko commented Feb 25, 2018

Leko commented Feb 26, 2018

0joshuaolson1 commented Feb 27, 2018 •

edited

Loading

BridgeAR left a comment

BridgeAR Mar 2, 2018

princejwesley Mar 2, 2018

BridgeAR Mar 2, 2018

princejwesley Mar 2, 2018

BridgeAR commented Mar 2, 2018

BridgeAR commented Mar 6, 2018

BridgeAR commented Mar 11, 2018 •

edited

Loading

repl: better handling of recoverable errors #18915

repl: better handling of recoverable errors #18915

Conversation

princejwesley commented Feb 21, 2018

Better handling of recoverable errors in REPL module

Checklist

Affected core subsystem(s)

Leko left a comment

Choose a reason for hiding this comment

Leko commented Feb 24, 2018

Leko commented Feb 25, 2018

Leko commented Feb 26, 2018

0joshuaolson1 commented Feb 27, 2018 • edited Loading

BridgeAR left a comment

Choose a reason for hiding this comment

BridgeAR Mar 2, 2018

Choose a reason for hiding this comment

princejwesley Mar 2, 2018

Choose a reason for hiding this comment

BridgeAR Mar 2, 2018

Choose a reason for hiding this comment

princejwesley Mar 2, 2018

Choose a reason for hiding this comment

BridgeAR commented Mar 2, 2018

BridgeAR commented Mar 6, 2018

BridgeAR commented Mar 11, 2018 • edited Loading

0joshuaolson1 commented Feb 27, 2018 •

edited

Loading

BridgeAR commented Mar 11, 2018 •

edited

Loading