Behavior of when is different when returning a rejected promise vs false

[simonmarklar]

trafficstars

Hi,

This maybe my misinterpretation of the docs but shouldn't the behavior of .when be the same for both a function returning true/false or Promise.resolve()/Promise.reject() ?

If a Resolved promise or true is returned from when the question is asked as expected.

If a rejected promise is returned from when it is caught in inquirer.promt().catch() and the console sits getting input and printing it to the screen but not moving to the next prompt and ctrl-c is needed to quit. if you return false the next prompt is displayed. I expect that in both cases the next prompt is displayed.

my setup: inquirer 3.0.1 node v7.0.0 Windows 10 Powershell

I have distilled my code down to 2 questions that illustrate what i mean:

'use strict'

const inquirer = require('inquirer')

const questions = [{
  name: 'overwrite',
  type: 'confirm',
  message: 'Configuration already present in config file. Are you sure you want to overwrite it',
  default: false,
  when: () => false // Promise.reject('failed')
}, {
  name: 'name',
  message: 'What is your name',
  type: 'input',
  when: (answers) => answers.overwrite || answers.overwrite === undefined
}]

inquirer.prompt(questions)
  .then((answers) => console.log('.then', answers))
  .catch((err) => console.log(`.catch ${err}`))

output with when: () => false:

? What is your name s
.then { name: 's' }

output with when: () => Promise.reject():

.catch failed

output with when: () => true:

? Configuration already present in config file. Are you sure you want to overwrite it Yes
? What is your name asd
.then { overwrite: true, name: 'asd' }

output with when: () => Promise.resolve('any value i can think of'):

? Configuration already present in config file. Are you sure you want to overwrite it Yes
? What is your name asd
.then { overwrite: true, name: 'asd' }

[simonmarklar]

I am happy to have a look at this over the weekend and submit a PR if that helps.

for now i am re-ordering my code to get around this

[SBoudrias]

Thanks, I believe we just don't handle the Promise failure in that way. When we moved from callbacks to promises, we only handled successful promise passing the status (true/false).

Rejected promise were treated as errors. So some code around the promise implementation might still follow this initial behavior.

[simonmarklar]

Hi, sorry i've not had the time to look at the code yet.

You're right, if i have when: () => Promise.resolve(false) then it behaves the same as returning false.

I checked the validate function for an input prompt and if i reject a promise in there it will re-ask the question rather than trigger the .catch function (i think this is why i was rejecting in when - I expected the behavior to be uniform).

Swallowing the rejected promise and making it act the same as Promise.resolve(false) does raise the question of what to do if the when function throws a legitimate error that should be handled by the developer using the library. I'm not sure if this happens, but if inquirer does swallow the rejection and behave as if the result is false, it could be a really frustrating bug to track down. So perhaps how it works now is better than swallowing a possible legit error, and just making the docs a little clearer would suffice (or perhaps this thread is enough clarification?)

I wonder about the clean up when a promise is rejected, because i dont think that just sitting there waiting for ctrl-c is what you intended. It looks like PromptUI.prototype.onCompletion is not being called when there is a rejected promise in this part of the code

I thought perhaps I should be cleaning up in inquirier.prompt().catch(), but the prompt's close function doesnt appear to be available.

as an experiment, I changed this part of prompt.js to

 return this.process
    .reduce(function (answers, answer) {
      _.set(this.answers, answer.name, answer.answer);
      return this.answers;
    }.bind(this), {})
    .toPromise(Promise)
    .then(this.onCompletion.bind(this))
    .catch((err) => {
      this.onCompletion()
      throw err
    });

and inquirer stops the prompt and triggers the .catch with the rejection error from when - so perhaps this is the solution to the hanging prompt. or maybe its better to expose a function on the Inquirer object that will run the prompts onCompletion function ?

I'm happy to turn the above snippet this into a PR if it is in line with how you want inquirer to work