hemerajs / hemera

🔬 Writing reliable & fault-tolerant microservices in Node.js https://hemerajs.github.io/hemera/
MIT License
806 stars 70 forks source link

async/await callback function #166

Closed veeramarni closed 7 years ago

veeramarni commented 7 years ago

Description

Need support for async/await callback function. It used to work in earlier version 1.5.6 and was failing in later versions.

I have written the test case for it and submitted PR #165 and right now it seems to be failing.

veeramarni commented 7 years ago

@StarpTech

Thanks for quick response. I was using async and await as you see here

https://github.com/veeramarni/hemera/blob/d2397715afae37f55a37745f4b332134e3a042a6/test/hemera/plugin.async-await.js#L93

https://github.com/veeramarni/hemera/blob/d2397715afae37f55a37745f4b332134e3a042a6/test/hemera/plugin.async-await.js#L79

StarpTech commented 7 years ago

@veeramarni you are using a wrong plugin signature. You have to use async or call the next handler. https://hemerajs.github.io/hemera/1_plugins.html

veeramarni commented 7 years ago

You mean like this. It still fails.

    // Plugin
    function plugin(hemera, options, next) {
      hemera.add(
        {
          topic: 'math',
          cmd: 'add'
        },
        async req => {
          const result = await Promise.resolve({
            result: req.a + req.b
          })
          return result
        }
      )

      next();
    }

    hemera.use({
      plugin: plugin,
      options: {
        name: 'myPlugin'
      }
    })
StarpTech commented 7 years ago

It works

  it.only('Should able to run async await callback function', function(done) {
    const nats = require('nats').connect(authUrl)

    const hemera = new Hemera(nats)

    // Plugin
    function plugin(hemera, options, next) {
      hemera.add(
        {
          topic: 'math',
          cmd: 'add'
        },
        async req => {
          const result = await Promise.resolve({
            result: req.a + req.b
          })
          return result
        }
      )
      next()
    }

    hemera.use({
      plugin: plugin,
      options: {
        name: 'myPlugin'
      }
    })
    hemera.ready(async () => {
      const resp = await hemera.act({
        topic: 'math',
        cmd: 'add',
        a: 1,
        b: 20
      })
      expect(resp.result).to.be.equals(21)
      hemera.close(done)
    })
  })

Do you using the latest hemera version?

StarpTech commented 7 years ago

@veeramarni could you verify it?

veeramarni commented 7 years ago

Sorry for the delay, when I was running npm run test to verify this change, I just get following errors. Not sure it was my laptop.

image

I lost familiarity with mocha as I was using jest for a while. What is the command to run to just run test on that file?

StarpTech commented 7 years ago

Requirements:

npm run test is correct. Could it be that you have running a nats server beside the tests? Hemera is tested on windows and Linux from nodejs 4-8 via CI.

pward123 commented 7 years ago

I ran into a similar problem running tests on my system. The issue is that when a test fails, the hemera-testsuite module does not kill the nats server even though the call is listed in after(). I manually fix this each time the tests fail by running the following on my mac:

kill $(lsof -n -i4TCP:6242 | grep LISTEN | awk '{print $2}')

StarpTech commented 7 years ago

@pward123 is the mocha after callback called?

pward123 commented 7 years ago

I believe so, yes

StarpTech commented 7 years ago

Can you verify it? When yes we should check the bootstrap script.

pward123 commented 7 years ago

After is not being called when hemera tests fail. As an experiment, I converted hemera.ready to an async function which means the tests can be structured like:

it('Should call after when failing inside hemera act response', async () => {
  const nats = require('nats').connect(authUrl)
  const hemera = new Hemera(nats)
  try {
    await hemera.ready()

    hemera.add(
      {
        topic: 'math',
        cmd: 'multiply'
      },
      (req, cb) => {
        cb(null, req.a * req.b)
      }
    )

    const resp = await hemera.act(
      {
        topic: 'math',
        cmd: 'multiply',
        a: 0,
        b: 0
      }
    )

    expect(resp).to.be.equals(-1)
  } finally {
    hemera.close()
  }
})

With this change, the after is called and the server is killed

pward123 commented 7 years ago

The issue is that the expect causes an exception in an async callback. You could also try/catch the 'expect' failure and call done(err) to get around the problem

StarpTech commented 7 years ago

The after callback isn't called without the try / catch block? It's correct that expect throws and your solution looks reasonable but you don't have to catch the assertion.

pward123 commented 7 years ago

I think that if the expect is inside a callback that there's no way for mocha to detect the exception. So you have to handle calling 'done' yourself or async/await the entire chain.

pward123 commented 7 years ago

This seems to describe what we're running into here. https://stackoverflow.com/a/26793989/1458832

StarpTech commented 7 years ago

yes, that's correct because mocha cant catch error which is thrown asynchronously but in your example above there is no callback.

StarpTech commented 7 years ago

The async test is executed asynchronously.

StarpTech commented 7 years ago

Closing due to inactivity.