REPL: Fix for multiline input when using custom eval function

[mikeybox]

Multiline input doesn't work if user overrides "eval" in
REPL.start. Overriding default eval will stop multiline
inputs on syntax errors because of the current error checking
in the finish callback.

This fixes that issue and allows for a custom recoverableError
check function to be passed the the REPL server for more
customisable use.

fixes #8640

[davidchambers]

I think this should read non-standard rather than none standard.

[mikeybox]

thanks @davidchambers updated the copy.

[thefourtheye]

Can we see an example to reproduce the problem?

[mikeybox]

Hey @thefourtheye this fix was submitted because it breaks multi-line support in Babel.js #1741

To reproduce the error you can simply use a custom eval function passed to the REPL server,
you will see that syntax errors are no longer caught in the finish method of the server.
The problem is the event upgrade that happens in the defaultEval handler is local to REPL so overriding this means that no syntax errors created in a custom eval will be promoted to Recoverable.

in the finish method bellow at line 300 it checks for errors to be an instance of Recoverable
this will never happen for errors thrown in a custom eval method.

// If error was SyntaxError and not JSON.parse error
      if (e) {
        if (e instanceof Recoverable) {
          // Start buffering data like that:
          // {
          // ...  x: 1
          // ... }
          self.bufferedCommand += cmd + '\n';
          self.displayPrompt();
          return;
        } else {
          self._domain.emit('error', e);
        }
      }

In the fix I also included a recoverable test option for instances where a custom eval might throw an error that is recoverable but the message is non-standard (this happens in the case of Babel) and will not be matched by isRecoverable

[jasnell]

@joyent/node-tsc ... this change proposes adding a new option, which is technically an API change, but it's for a good reason. Thoughts on this one?

[cjihrig]

This would be better as a ternary based on whether options.recoverable is a function or not.

[cjihrig]

@jasnell I left a few comments. I don't really have an opinion on whether this gets in or not.

[mikeybox]

@cjihrig I'll add your suggestions to the PR, in terms of testing you're right it isn't really tested but neither was the eval function (unless I'm missing something??) so I just kept it in line with that as it directly affects that.

[smasher164]

@mikeybox Here's the relevant issue on nodejs/node#2939

[CestDiego]

are there any news on this?

[mikeybox]

@CestDiego there is a similar fix in a PR which is further along nodejs/node#3488.