Improve error message for bad app.use arguments

History.md

@@ -2,6 +2,7 @@ unreleased
 ==========
 
   * Fix regression for empty string `path` in `app.use`
+  * Improve error message for bad `app.use` arguments
 
 4.9.1 / 2014-09-16
 ==================

lib/router/index.js

@@ -1,3 +1,4 @@
+
 /**
  * Module dependencies.
  */
@@ -8,9 +9,15 @@ var methods = require('methods');
 var mixin = require('utils-merge');
 var debug = require('debug')('express:router');
 var parseUrl = require('parseurl');
+var utils = require('../utils');
+
+/**
+ * Module variables.
+ */
+
+var objectRegExp = /^\[object (\S+)\]$/;
 var slice = Array.prototype.slice;
 var toString = Object.prototype.toString;
-var utils = require('../utils');
 
 /**
  * Initialize a new `Router` with the given `options`.
@@ -413,14 +420,12 @@ proto.use = function use(fn) {
   var callbacks = utils.flatten(slice.call(arguments, offset));
 
   if (callbacks.length === 0) {
-    throw new TypeError('Router.use() requires callback function');
+    throw new TypeError('Router.use() requires middleware functions');
   }
 
   callbacks.forEach(function (fn) {
     if (typeof fn !== 'function') {
-      var type = toString.call(fn);
-      var msg = 'Router.use() requires callback function but got a ' + type;
-      throw new TypeError(msg);
+      throw new TypeError('Router.use() requires middleware function but got a ' + gettype(fn));
     }
 
     // add the middleware
@@ -477,6 +482,19 @@ methods.concat('all').forEach(function(method){
   };
 });
 
+// get type for error message
+function gettype(obj) {
+  var type = typeof obj;
+
+  if (type !== 'object') {
+    return type;
+  }
+
+  // inspect [[Class]] for objects
+  return toString.call(obj)
+    .replace(objectRegExp, '$1');
+}
+
 // merge params with parent params
 function mergeParams(params, parent) {
   if (typeof parent !== 'object' || !parent) {

test/Router.js

@@ -300,12 +300,15 @@ describe('Router', function(){
   describe('.use', function() {
     it('should require arguments', function(){
       var router = new Router();
-      router.use.bind(router).should.throw(/requires callback function/)
+      router.use.bind(router).should.throw(/requires middleware function/)
     })
 
     it('should not accept non-functions', function(){
       var router = new Router();
-      router.use.bind(router, '/', 'hello').should.throw(/requires callback function/)
+      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/)
     })
   })
 

test/app.use.js

@@ -16,11 +16,6 @@ describe('app', function(){
     app.use(blog);
   })
 
-  it('should reject missing functions', function(){
-    var app = express();
-    app.use.bind(app, 3).should.throw(/requires middleware function/);
-  })
-
   describe('.use(app)', function(){
     it('should mount the app', function(done){
       var blog = express()
@@ -258,6 +253,19 @@ 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 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 strip path from req.url', function (done) {
       var app = express();