Skip to content

Commit

Permalink
Fix: don't swallow send error, send error has precedence over other e…
Browse files Browse the repository at this point in the history
…xtension errors
  • Loading branch information
StarpTech committed Aug 5, 2018
1 parent 9a9fd1b commit 82cc1c7
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 7 deletions.
49 changes: 49 additions & 0 deletions examples/basic/extensions.js
@@ -0,0 +1,49 @@
'use strict'

const Hemera = require('./../../packages/hemera')
const nats = require('nats').connect()

const hemera = new Hemera(nats, {
logLevel: 'info'
})

hemera.ready(() => {
hemera.ext('onServerPreRequest', function(hemera, request, reply, next) {
console.log('onServerPreRequest')
next()
})
hemera.ext('onServerPreHandler', function(hemera, request, reply, next) {
console.log('onServerPreHandler')
next()
})
hemera.ext('onServerPreResponse', function(hemera, request, reply, next) {
console.log('onServerPreResponse')
next()
})

hemera.add(
{
topic: 'math',
cmd: 'add'
},
function(req, cb) {
cb(null, req.a + req.b)
}
)

hemera.act(
{
topic: 'math',
cmd: 'add',
a: 1,
b: 20
},
function(err, resp) {
if (err) {
this.log.error(err)
return
}
this.log.info(resp)
}
)
})
2 changes: 1 addition & 1 deletion packages/hemera/lib/index.js
Expand Up @@ -236,7 +236,7 @@ class Hemera extends EventEmitter {
* @memberof Hemera
*/
_registerErrors() {
for (var error in Hemera.errors) {
for (let error in Hemera.errors) {
Errio.register(Hemera.errors[error])
}
}
Expand Down
4 changes: 3 additions & 1 deletion packages/hemera/lib/reply.js
Expand Up @@ -148,7 +148,9 @@ class Reply {
).causedBy(extensionError)
self.log.error(internalError)
self.hemera.emit('serverResponseError', extensionError)
self.send(extensionError)
// don't use send() here in order to avoid rexecution of serverPreResponse
// and to send the "send-error" as final response
self.error = extensionError
}

if (self._response.replyTo) {
Expand Down
6 changes: 5 additions & 1 deletion test/hemera/async-schemaCompiler.spec.js
Expand Up @@ -154,7 +154,11 @@ describe('Async Schema Compiler', function() {
(err, resp) => {
expect(err).to.be.exists()
expect(err.name).to.be.equals('TimeoutError')
hemera.close(done)
// give the promise the chance to respond within active nats connection
// otherwise we will run into a connection error
setTimeout(() => {
hemera.close(done)
}, 200)
}
)
})
Expand Down
106 changes: 102 additions & 4 deletions test/hemera/server-extensions.spec.js
Expand Up @@ -333,7 +333,7 @@ describe('Server Extensions', function() {
})
})

it('Should not be able to send the payload in onServerPreResponse because payload was already set', function(done) {
it('Should not be able to send the payload in onServerPreResponse because success payload was already set', function(done) {
let ext = Sinon.spy()

const nats = require('nats').connect(authUrl)
Expand Down Expand Up @@ -376,7 +376,103 @@ describe('Server Extensions', function() {
})
})

it('Should not be able send the extension error in onServerPreResponse because payload was already set', function(done) {
it('Should send the onServerPreRequest error instead the last extension error payload', function(done) {
let onServerPreRequestSpy = Sinon.spy()
let onServerPreHandlerSpy = Sinon.spy()

const nats = require('nats').connect(authUrl)

const hemera = new Hemera(nats)

hemera.ready(() => {
hemera.ext('onServerPreRequest', function(ctx, req, res, next) {
onServerPreRequestSpy()
next(new Error('test1'))
})
hemera.ext('onServerPreHandler', function(ctx, req, res, next) {
onServerPreHandlerSpy()
next(new Error('test2'))
})

hemera.add(
{
topic: 'email',
cmd: 'send'
},
(resp, cb) => {
cb(null, true)
}
)

hemera.act(
{
topic: 'email',
cmd: 'send',
email: 'foobar@gmail.com',
msg: 'Hi!'
},
(err, resp) => {
expect(err).to.be.exists()
expect(err.name).to.be.equals('Error')
expect(err.message).to.be.equals('test1')
expect(resp).to.be.undefined()
expect(onServerPreRequestSpy.called).to.be.equals(true)
expect(onServerPreHandlerSpy.called).to.be.equals(false)
hemera.close(done)
}
)
})
})

it('Should send the extension error in onServerPreResponse instead previous extensions errors', function(done) {
let onServerPreHandlerSpy = Sinon.spy()
let onServerPreResponse = Sinon.spy()

const nats = require('nats').connect(authUrl)

const hemera = new Hemera(nats)

hemera.ready(() => {
hemera.ext('onServerPreHandler', function(ctx, req, res, next) {
onServerPreHandlerSpy()
next(new Error('test1'))
})
hemera.ext('onServerPreResponse', function(ctx, req, res, next) {
onServerPreResponse()
next(new Error('test2'))
})

hemera.add(
{
topic: 'email',
cmd: 'send'
},
(resp, cb) => {
cb(null, true)
}
)

hemera.act(
{
topic: 'email',
cmd: 'send',
email: 'foobar@gmail.com',
msg: 'Hi!'
},
(err, resp) => {
expect(err).to.be.exists()
expect(err.name).to.be.equals('Error')
expect(err.message).to.be.equals('test2')
expect(resp).to.be.undefined()
expect(onServerPreHandlerSpy.called).to.be.equals(true)
expect(onServerPreResponse.called).to.be.equals(true)
hemera.close(done)
}
)
})
})

it('Should send the extension error in onServerPreResponse instead the success payload', function(done) {
let ext = Sinon.spy()

const nats = require('nats').connect(authUrl)
Expand Down Expand Up @@ -407,8 +503,10 @@ describe('Server Extensions', function() {
msg: 'Hi!'
},
(err, resp) => {
expect(err).to.be.not.exists()
expect(resp).to.be.equals(true)
expect(err).to.be.exists()
expect(err.name).to.be.equals('Error')
expect(err.message).to.be.equals('test')
expect(resp).to.be.undefined()
expect(ext.called).to.be.equals(true)
hemera.close(done)
}
Expand Down

0 comments on commit 82cc1c7

Please sign in to comment.