Skip to content

Commit

Permalink
Improve error messages when non-function provided as middleware
Browse files Browse the repository at this point in the history
closes #3426
  • Loading branch information
shime authored and dougwilson committed Sep 28, 2017
1 parent 12c3712 commit 2df1ad2
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 23 deletions.
1 change: 1 addition & 0 deletions History.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ unreleased
==========

* Improve error message when autoloading invalid view engine
* Improve error messages when non-function provided as middleware
* Skip `Buffer` encoding when not generating ETag for small response
* Use `safe-buffer` for improved Buffer API
* deps: accepts@~1.3.4
Expand Down
2 changes: 1 addition & 1 deletion lib/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ app.use = function use(fn) {
var fns = flatten(slice.call(arguments, offset));

if (fns.length === 0) {
throw new TypeError('app.use() requires middleware functions');
throw new TypeError('app.use() requires a middleware function')
}

// setup router
Expand Down
4 changes: 2 additions & 2 deletions lib/router/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,14 +448,14 @@ proto.use = function use(fn) {
var callbacks = flatten(slice.call(arguments, offset));

if (callbacks.length === 0) {
throw new TypeError('Router.use() requires middleware functions');
throw new TypeError('Router.use() requires a middleware function')
}

for (var i = 0; i < callbacks.length; i++) {
var fn = callbacks[i];

if (typeof fn !== 'function') {
throw new TypeError('Router.use() requires middleware function but got a ' + gettype(fn));
throw new TypeError('Router.use() requires a middleware function but got a ' + gettype(fn))
}

// add the middleware
Expand Down
4 changes: 2 additions & 2 deletions lib/router/route.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ Route.prototype.all = function all() {

if (typeof handle !== 'function') {
var type = toString.call(handle);
var msg = 'Route.all() requires callback functions but got a ' + type;
var msg = 'Route.all() requires a callback function but got a ' + type
throw new TypeError(msg);
}

Expand All @@ -198,7 +198,7 @@ methods.forEach(function(method){

if (typeof handle !== 'function') {
var type = toString.call(handle);
var msg = 'Route.' + method + '() requires callback functions but got a ' + type;
var msg = 'Route.' + method + '() requires a callback function but got a ' + type
throw new Error(msg);
}

Expand Down
30 changes: 21 additions & 9 deletions test/Router.js
Original file line number Diff line number Diff line change
Expand Up @@ -368,17 +368,29 @@ describe('Router', function(){
})

describe('.use', function() {
it('should require arguments', function(){
var router = new Router();
router.use.bind(router).should.throw(/requires middleware function/)
it('should require middleware', function () {
var router = new Router()
assert.throws(function () { router.use('/') }, /requires a middleware function/)
})

it('should not accept non-functions', function(){
var router = new Router();
router.use.bind(router, '/', 'hello').should.throw(/requires middleware function.*string/)
router.use.bind(router, '/', 5).should.throw(/requires middleware function.*number/)
router.use.bind(router, '/', null).should.throw(/requires middleware function.*Null/)
router.use.bind(router, '/', new Date()).should.throw(/requires middleware function.*Date/)
it('should reject string as middleware', function () {
var router = new Router()
assert.throws(function () { router.use('/', 'foo') }, /requires a middleware function but got a string/)
})

it('should reject number as middleware', function () {
var router = new Router()
assert.throws(function () { router.use('/', 42) }, /requires a middleware function but got a number/)
})

it('should reject null as middleware', function () {
var router = new Router()
assert.throws(function () { router.use('/', null) }, /requires a middleware function but got a Null/)
})

it('should reject Date as middleware', function () {
var router = new Router()
assert.throws(function () { router.use('/', new Date()) }, /requires a middleware function but got a Date/)
})

it('should be called for any URL', function (done) {
Expand Down
31 changes: 22 additions & 9 deletions test/app.use.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@

var after = require('after');
var assert = require('assert')
var express = require('..');
var request = require('supertest');

Expand Down Expand Up @@ -253,17 +254,29 @@ describe('app', function(){
})

describe('.use(path, middleware)', function(){
it('should reject missing functions', function () {
var app = express();
app.use.bind(app, '/').should.throw(/requires middleware function/);
it('should require middleware', function () {
var app = express()
assert.throws(function () { app.use('/') }, /requires a middleware function/)
})

it('should reject non-functions as middleware', function () {
var app = express();
app.use.bind(app, '/', 'hi').should.throw(/requires middleware function.*string/);
app.use.bind(app, '/', 5).should.throw(/requires middleware function.*number/);
app.use.bind(app, '/', null).should.throw(/requires middleware function.*Null/);
app.use.bind(app, '/', new Date()).should.throw(/requires middleware function.*Date/);
it('should reject string as middleware', function () {
var app = express()
assert.throws(function () { app.use('/', 'foo') }, /requires a middleware function but got a string/)
})

it('should reject number as middleware', function () {
var app = express()
assert.throws(function () { app.use('/', 42) }, /requires a middleware function but got a number/)
})

it('should reject null as middleware', function () {
var app = express()
assert.throws(function () { app.use('/', null) }, /requires a middleware function but got a Null/)
})

it('should reject Date as middleware', function () {
var app = express()
assert.throws(function () { app.use('/', new Date()) }, /requires a middleware function but got a Date/)
})

it('should strip path from req.url', function (done) {
Expand Down

0 comments on commit 2df1ad2

Please sign in to comment.