2

I've currently got an ES6 class with a constructor and two methods. I'm a tad confused as to why using Promise.reject(ex) within the .then() is resolving undefined. If someone wouldn't mind explaining what I'm doing wrong that would be much appreciated.

I have a method called getYaml() which contains the following:

_getYaml(recordId) {
  return new Promise((resolve, reject) => {
    fs.readFile(this.workingDir + '/' + recordId + '.yaml', 'utf8', function(err, data) {
      if (err) reject(err)
      resolve(data)
    })
  })
}

I then have another method called getCompDoc which makes use of the other method like so:

getCompDoc(recordId) {
  return this._getYaml(recordId).then(data => {

    let yaml = data

    let yamlObj
    try {
      yamlObj = YAML.safeLoad(yaml)
    } catch (ex) {
      ex.message = `Failure to parse yaml. Error: ${ex.message}`
      logger.error(ex.message, {}, ex)
      return Promise.reject(ex)
    }

    let compDoc = {
      // ...
    }

    return compDoc

  }).catch(err => {
    logger.error(err, {}, err)
  })
}

I then have a test to check that the YAML parsing error is caught and then a promise rejected which looks like so:

describe('error cases', () => {
  const fakeRecordId = 'SomeYaml'
  beforeEach(() => {
    sinon.stub(myClass, '_getYaml').returns(Promise.resolve('{{&^%}egrinv&alidgj%^%^&$£@£@£}'))
  })

  afterEach(() => {
    myClass._getYaml.restore()
  })

  it('Error parsing yaml, rejects with error', () => {
    return expect(myClass.getCompDoc(fakeRecordId)).to.be.rejected.then(response => {
      expect(response.message).to.match(/Failure to parse yaml. Error: /)
    })
  })
})

Test output:

AssertionError: expected promise to be rejected but it was fulfilled with undefined

If I simply return the exception that is thrown within the getCompDoc method, I recieve the error as expected, however as soon as I use Promise.reject it resolves with undefined.

I was thinking of wrapping the getCompDoc in a return new Promise() however I wasn't sure if this would be an example of the Promise constructor anti-pattern. I would ideally like to reject this, instead of returning the error directly as then I can assert that the method was rejected and not fulfilled.

Matt Kent
  • 1,145
  • 1
  • 11
  • 26
  • 1
    I'm not sure why you don't allow the exception to fall into your catch instead of wrapping in a try/catch in getCompDoc e.g. `const yamlObj = await YAML.safeLoad(yaml)`? – Dominic Apr 23 '19 at 14:39
  • Throwing `ex` gives me the same assertion error – Matt Kent Apr 23 '19 at 14:41
  • 3
    `}).catch(err => { logger.error(err, {}, err) })` catches all errors, you would need to rethrow it i guess. – Roland Starke Apr 23 '19 at 14:41

1 Answers1

2

You 'swallow' the error in getCompDoc in your catch clause. Specifically, here's a simplified snippet representing your code:

let getYamlPromise = Promise.reject('REJECTED!');

let getCompDocPromise = getYamlPromise
    .then(data => console.log('getYamlPromise', data))
    .catch(error => console.error('getYamlPromise', error));

getCompDocPromise
    .then(a => console.log('getCompDocPromise RESOLVED', a))
    .catch(a => console.log('getCompDocPromise REJECTED', a));

As you can see, getCompDocPromise is resolved with undefined. If you would like to propagate the error, your catch clause will have to throw a new error or return a rejected promise:

let getYamlPromise = Promise.reject('REJECTED!');

let getCompDocPromise = getYamlPromise
    .then(data => console.log('getYamlPromise', data))
    .catch(error => {
      console.error('getYamlPromise', error);
      return Promise.reject(error);
    });

getCompDocPromise
    .then(a => console.log('getCompDocPromise RESOLVED', a))
    .catch(a => console.log('getCompDocPromise REJECTED', a));
junvar
  • 11,151
  • 2
  • 30
  • 46