Proposal: Hemera.add should throw BusinessError if promise-based handler rejects

[pward123]

Description

It would be nice if returning a promise from a Hemera.add handler worked the same as using a callback. For example, I would expect both services below to behave identically.

hemera.ready(() => {
  let Joi = hemera.joi

  const myPromise = (fail) => (
    new Promise((resolve, reject) => {
      if (fail) {
        return reject(new Error('boom'))
      }
      resolve(42)
    })
  )

  hemera.add({
    topic: 'test',
    cmd: 'boom',
    fail: Joi.boolean().required(),
  }, function ({fail}, cb) {
    return asCallback(myPromise(fail), cb)
  })

  hemera.add({
    topic: 'test',
    cmd: 'boom2',
    fail: Joi.boolean().required(),
  }, function ({fail}) {
    return myPromise(fail)
  })

  // const cmd = 'boom'
  const cmd = 'boom2'

  hemera
    .act({ topic: 'test', cmd, fail: true })
    .then(x => hemera.log.info(x))
    .catch(e => console.log(e.stack))
})

Steps to Reproduce

1. Clone https://github.com/pward123/hemera-add-promise-test.git
2. cd hemera-add-promise-test
3. yarn install
4. npm start

Expected Result

The catch should immediately show a BusinessError

Actual Result

The console shows a deprecation warning for an unhandled promise rejection and the catch shows a Timeout error after the timeout expires.

Context

• nats-hemera (core)
• hemera-zipkin
• hemera-store
• hemera-stats
• hemera-rabbitmq
• hemera-nsq
• hemera-arango-store
• hemera-sql-store
• hemera-elasticsearch
• hemera-couchbase-store
• hemera-mongo-store
• hemera-joi
• hemera-parambulator
• hemera-msgpack
• hemera-avro
• hemera-redis-cache
• hemera-jwt-auth

Your Environment

• NATS: 0.7.20
• Hemera: 2.1.0
• NodeJs: 8.6.0
• macos 10.12.6

[pward123]

In the example above asCallback is from https://www.npmjs.com/package/ascallback

[StarpTech]

If I understand your case correctly you want to support to pass promises to callback handler or return them inside a normal function?

When yes this is definitely wrong because there are only two types of asynchronous composition which are very common in javascript.

• Callback style

  hemera.add({
    topic: 'test',
    cmd: 'boom',
    fail: Joi.boolean().required(),
  }, function (req, cb) {
    cb(<error>, <result>) // result can be any value, error must be an instance of Error
  })

• Async / Await

  hemera.add({
    topic: 'test',
    cmd: 'boom',
    fail: Joi.boolean().required(),
  }, async function (req, cb) {
    const result = await Promise.resolve()
    return result // can be a promise or any other value
  })

• Pass promise as value

Some others support this syntax but we don't provide a reply interface in an act or add

  hemera.add({
    topic: 'test',
    cmd: 'boom',
    fail: Joi.boolean().required(),
  }, async function (req, reply) {
       reply.send(Promise.resolve())
  })

This is possible in hemera.

      hemera
        .act(
          {
            topic: 'math',
            cmd: 'add',
            a: 1,
            b: 2
          })
          .then(..)
          .catch(..)

Could I answer your question?

[StarpTech]

You have to decide between callback or async / await don't mix them.

[pward123]

Yep, if you want to use promises with hemera, return an async function or use asCallback.

[StarpTech]

Could I answer your question?

[pward123]

Yes, I believe so. Thanks.

I've always been under the impression that (from the callers perspective) both of the following just return a promise. I guess there are some subtleties that I haven't ran into until now.

async () => {
  return await myPromise()
}

() => {
  return myPromise()
}

[StarpTech]

No, this is quite uncommon.