Skip to content

Commit

Permalink
fix: make arity error message actionable (#1901)
Browse files Browse the repository at this point in the history
  • Loading branch information
josephharrington committed Apr 28, 2022
1 parent 05f12a6 commit 97b6f93
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 12 deletions.
23 changes: 21 additions & 2 deletions lib/chain.js
Expand Up @@ -73,12 +73,31 @@ Chain.prototype.getHandlers = function getHandlers() {
*/
Chain.prototype.add = function add(handler) {
assert.func(handler);
var handlerId = handler._identifier || handler._name || handler.name;
if (handler.length <= 2) {
// arity <= 2, must be AsyncFunction
assert.equal(handler.constructor.name, 'AsyncFunction');
assert.equal(
handler.constructor.name,
'AsyncFunction',
`Handler [${handlerId}] is missing a third argument (the ` +
'"next" callback) but is not an async function. Middlware ' +
'handlers can be either async/await or callback-based.' +
'Callback-based (non-async) handlers should accept three ' +
'arguments: (req, res, next). Async handler functions should ' +
'accept maximum of 2 arguments: (req, res).'
);
} else {
// otherwise shouldn't be AsyncFunction
assert.notEqual(handler.constructor.name, 'AsyncFunction');
assert.notEqual(
handler.constructor.name,
'AsyncFunction',
`Handler [${handlerId}] accepts a third argument (the 'next" ` +
'callback) but is also an async function. Middlware handlers ' +
'can be either async/await or callback-based. Async handler ' +
'functions should accept maximum of 2 arguments: (req, res). ' +
'Non-async handlers should accept three arguments: (req, ' +
'res, next).'
);
}

// _name is assigned in the server and router
Expand Down
2 changes: 2 additions & 0 deletions lib/router.js
Expand Up @@ -205,6 +205,8 @@ Router.prototype.mount = function mount(opts, handlers) {
handler._name =
handler.name || 'handler-' + self._anonymousHandlerCounter++;

handler._identifier = `${handler._name} on ${opts.method} ${opts.path}`;

// Attach to middleware chain
chain.add(handler);
});
Expand Down
21 changes: 11 additions & 10 deletions test/chain.test.js
Expand Up @@ -409,23 +409,24 @@ test('abort with throw inside async function', function(t) {
});

test('fails to add non async function with arity 2', function(t) {
var handler = function getLunch(req, res) {
res.send('ok');
};
var chain = new Chain();

t.throws(function() {
chain.add(function(req, res) {
res.send('ok');
});
}, Error);
chain.add(handler);
}, /getLunch/);
t.end();
});

test('fails to add async function with arity 3', function(t) {
var chain = new Chain();
var handler = async function getBreakfast(req, res, next) {
res.send('ok');
};

var chain = new Chain();
t.throws(function() {
chain.add(async function(req, res, next) {
res.send('ok');
});
}, Error);
chain.add(handler);
}, /getBreakfast/);
t.end();
});

0 comments on commit 97b6f93

Please sign in to comment.