Rejected promises log `true`

[devinus]

vorpal
.command('fail me <arg>')
.description('Must return an arg.')
.action(function (args) {
  return new Promise(function (resolve, reject) {
    if (args.arg === 'not') {
      resolve('we are happy');
    } else {
      reject('we are not happy.');
    }
  });
});

$ fail me

  Missing required argument. Showing Help:

  Usage: fail me [options] <arg>

  Must return an arg.

  Options:

    --help  output usage information

$ fail me foo
true

[devinus]

This happens here: https://github.com/dthree/vorpal/blob/906cf7bc7728a6c15190815dd336e79e8ef2bc5a/lib/session.js#L432

response is { error: true, data: 'we are not happy.', args: undefined }.

Perhaps error.data is what should be logged?

[devinus]

And only when error.data is truthy? Unsure. Seems like it should only this.log if it's an actual instanceof Error.

[scotthovestadt]

@devinus Can you create a failing test? If there's a bug with a failing test I will fix it.

[zakhenry]

See the catch block where the promise rejection is handled. https://github.com/dthree/vorpal/blob/62f77e1477ec11245c320b5d1d56b521c36ea6d8/lib/session.js#L482-488

The second param of onCompletion (data) is set to true, which is what ends up being output. It's worth noting that a non-promise callback that throws new Error will cause an exit 1. This can be replicated by replacing the onCompletion with timeout to throw error out of the promise chain.

if (res && _.isFunction(res.then)) {
    res.then(function (data) {
      onCompletion(wrapper, undefined, data);
    }).catch(function (err) {
      // onCompletion(wrapper, true, err);
      setTimeout(() => {
        throw err
      });
    });
  }

I'm still not sure on what the intended behaviour is though, but exiting with 0 and logging 'true' cant be it.

[weinma]

Hi! My personal workaround for now:

I changed the onCompletet function in session.js:444 to:

function onCompletion(wrapper, err, data, argus) {
    response = {
      error: err ? data : null,
      data: data,
      args: argus
    };
    self.completeCommand();
  }

Note: I needed a workaround fast, so i didn't get completely into it. I just needed it to work for my cause.

[ahz]

According to comments in #35 the workaround can also be to reject promises with new Error().

vorpal
.command('fail me <arg>')
.description('Must return an arg.')
.action(function (args) {
  return new Promise(function (resolve, reject) {
    if (args.arg === 'not') {
      resolve('we are happy');
    } else {
      reject(new Error('we are not happy.')); // like this
    }
  });
});