Promise support? about rascal

[cressie176]

Release as 4.0.0

[cressie176]

Currently available on the promise-support branch. I'll publish once I've done some more testing and had it reviewed.

[cressie176]

Good spot. It's a weakness in the rascal API that you can't safely go async between calling subscribe and registering the on message handler.

Couple of things I might be able to do...

1. Accept an optional event handler as part of the subscribe function.
2. Not start consuming messages until an on message event listener was added using the newListener event

[cressie176]

I think the API will be better if the user can subscribe to events before starting to consume messages from rabbit, so I'm still giving it some thought. I have a lot of other stuff on at the moment though so it won't be soon though.

[cressie176]

@BorePlusPlus just came up with a nice idea. Instead of consuming messages immediately, if rascal could wait for an on('message') handler to be registered. Will investigate

[cressie176]

Thanks for the nudge @mixecan. I'll bump it up my list.

[cressie176]

No. It would increase the complexity of the code base and testing overhead.

[breezewish]

Could you expose classes via module.exports (for example, Broker) so that I can use bluebird.promisifyAll to provide a promise-style API?

[cressie176]

I'll create a branch and do it there. You happy to test / feedback?

[breezewish]

Thanks! It won't be better!
I will use it in our experimental environment :P

[cressie176]

Try

{
  "dependencies": {
    "rascal": "git://github.com/guidesmiths/rascal.git#promises"
  }
}

Promise.promisifyAll(require('rascal').classes)

[breezewish]

Thanks for your effort. However it doesn't fully work at least for broker (I haven't tried other classes yet) because:

1. Broker does not expose its base class https://github.com/guidesmiths/rascal/blob/promises/lib/amqp/Broker.js#L17
   In this way, broker instance methods such as broker.publish won't be promisified.

2. Broker does not use prototype to define its methods thus bluebird.promisifyAll(Object.getPrototypeOf(someBrokerInstance)); will not work either.

promisify(someBrokerInstance) works but it is not ideal because it is not a one-time setup.

I would try to make some changes and will send you PRs whose implementations are friendly to promisify.

[cressie176]

Thanks. I'm happy to accept a PR.

[breezewish]

I made first attempt to make Broker promisify friendly:

breezewish@c62c4a4?w=1

Simple example rewritten in Promise way and ES 2017:

import bluebird from 'bluebird'
import Rascal from 'rascal'

bluebird.promisifyAll(Rascal.Broker)
bluebird.promisifyAll(Rascal.Broker.prototype)

(async () => {

  const broker = await Rascal.Broker.createAsync(Rascal.withDefaultConfig(config))
  broker.on('error', console.error)

  const subscription = await broker.subscribeAsync('demo_sub')
  subscription
    .on('message', function(message, content, ackOrNack) {
      console.log(content)
      ackOrNack()
    })
    .on('error', console.error)

  setInterval(async () => {
    await publication = broker.publishAsync('demo_pub', new Date().toISOString() + ': hello world')
    publication.on('error', console.error)
  }, 1000)

}())

[breezewish]

I am going to test the new broker these days to see if there are any problems :P I will make PR later

[cressie176]

Looks good to me. Thanks. After submitting the PR how do you feel about maintaining a 'rascal-as-promised' module?

[breezewish]

Thanks, I might not be a good person to maintain that since most of the time I am not working with something related to AMQP :P I prefer to making rascal friendly to promisify tools so that users who want to use a promise style API can easily achieve that while rascal needn't care much about the promise API.

[breezewish]

Ok seems that the promise way may lose message..caused by await broker.subscribeAsync('demo_sub')..

[cressie176]

Not as easy as I'd hoped.

1. Would need to take multiple event handlers ('message', 'invalid_message', 'error', 'redeliveries_exceeded')
2. Breaks the subscrition.cancel behaviour.

[breezewish]

Agree with you. BTW Promise is at least a microtask so that changing API is unavoidable to support Promise. This functionality is more difficult than I thought :P

[mixecan]

@BorePlusPlus just came up with a nice idea. Instead of consuming messages immediately, if rascal could wait for an on('message') handler to be registered. Will investigate

Any change we can see this on the master branch?