Deprecate string and non-integer arguments to res.status

closes #4223

Author: jonchurch

Diff for: History.md

@@ -1,6 +1,7 @@
 unreleased
 ==========
 
+  * Deprecate string and non-integer arguments to `res.status`
   * Ignore `Object.prototype` values in settings through `app.set`/`app.get`
   * Support proper 205 responses using `res.send`
 

Diff for: lib/response.js

@@ -64,6 +64,9 @@ var charsetRegExp = /;\s*charset\s*=/;
  */
 
 res.status = function status(code) {
+  if ((typeof code === 'string' || Math.floor(code) !== code) && code > 99 && code < 1000) {
+    deprecate('res.status(' + JSON.stringify(code) + '): use res.status(' + Math.floor(code) + ') instead')
+  }
   this.statusCode = code;
   return this;
 };

Diff for: test/res.status.js

@@ -1,21 +1,202 @@
 'use strict'
 
 var express = require('../')
-  , request = require('supertest');
+var request = require('supertest')
 
-describe('res', function(){
-  describe('.status(code)', function(){
-    it('should set the response .statusCode', function(done){
-      var app = express();
+var isIoJs = process.release
+  ? process.release.name === 'io.js'
+  : ['v1.', 'v2.', 'v3.'].indexOf(process.version.slice(0, 3)) !== -1
 
-      app.use(function(req, res){
-        res.status(201).end('Created');
-      });
+describe('res', function () {
+  describe('.status(code)', function () {
+    describe('when "code" is undefined', function () {
+      it('should raise error for invalid status code', function (done) {
+        var app = express()
 
-      request(app)
-      .get('/')
-      .expect('Created')
-      .expect(201, done);
+        app.use(function (req, res) {
+          res.status(undefined).end()
+        })
+
+        request(app)
+          .get('/')
+          .expect(500, /Invalid status code/, function (err) {
+            if (isIoJs) {
+              done(err ? null : new Error('expected error'))
+            } else {
+              done(err)
+            }
+          })
+      })
+    })
+
+    describe('when "code" is null', function () {
+      it('should raise error for invalid status code', function (done) {
+        var app = express()
+
+        app.use(function (req, res) {
+          res.status(null).end()
+        })
+
+        request(app)
+          .get('/')
+          .expect(500, /Invalid status code/, function (err) {
+            if (isIoJs) {
+              done(err ? null : new Error('expected error'))
+            } else {
+              done(err)
+            }
+          })
+      })
+    })
+
+    describe('when "code" is 201', function () {
+      it('should set the response status code to 201', function (done) {
+        var app = express()
+
+        app.use(function (req, res) {
+          res.status(201).end()
+        })
+
+        request(app)
+          .get('/')
+          .expect(201, done)
+      })
+    })
+
+    describe('when "code" is 302', function () {
+      it('should set the response status code to 302', function (done) {
+        var app = express()
+
+        app.use(function (req, res) {
+          res.status(302).end()
+        })
+
+        request(app)
+          .get('/')
+          .expect(302, done)
+      })
+    })
+
+    describe('when "code" is 403', function () {
+      it('should set the response status code to 403', function (done) {
+        var app = express()
+
+        app.use(function (req, res) {
+          res.status(403).end()
+        })
+
+        request(app)
+          .get('/')
+          .expect(403, done)
+      })
+    })
+
+    describe('when "code" is 501', function () {
+      it('should set the response status code to 501', function (done) {
+        var app = express()
+
+        app.use(function (req, res) {
+          res.status(501).end()
+        })
+
+        request(app)
+          .get('/')
+          .expect(501, done)
+      })
+    })
+
+    describe('when "code" is "410"', function () {
+      it('should set the response status code to 410', function (done) {
+        var app = express()
+
+        app.use(function (req, res) {
+          res.status('410').end()
+        })
+
+        request(app)
+          .get('/')
+          .expect(410, done)
+      })
+    })
+
+    describe('when "code" is 410.1', function () {
+      it('should set the response status code to 410', function (done) {
+        var app = express()
+
+        app.use(function (req, res) {
+          res.status(410.1).end()
+        })
+
+        request(app)
+          .get('/')
+          .expect(410, function (err) {
+            if (isIoJs) {
+              done(err ? null : new Error('expected error'))
+            } else {
+              done(err)
+            }
+          })
+      })
+    })
+
+    describe('when "code" is 1000', function () {
+      it('should raise error for invalid status code', function (done) {
+        var app = express()
+
+        app.use(function (req, res) {
+          res.status(1000).end()
+        })
+
+        request(app)
+          .get('/')
+          .expect(500, /Invalid status code/, function (err) {
+            if (isIoJs) {
+              done(err ? null : new Error('expected error'))
+            } else {
+              done(err)
+            }
+          })
+      })
+    })
+
+    describe('when "code" is 99', function () {
+      it('should raise error for invalid status code', function (done) {
+        var app = express()
+
+        app.use(function (req, res) {
+          res.status(99).end()
+        })
+
+        request(app)
+          .get('/')
+          .expect(500, /Invalid status code/, function (err) {
+            if (isIoJs) {
+              done(err ? null : new Error('expected error'))
+            } else {
+              done(err)
+            }
+          })
+      })
+    })
+
+    describe('when "code" is -401', function () {
+      it('should raise error for invalid status code', function (done) {
+        var app = express()
+
+        app.use(function (req, res) {
+          res.status(-401).end()
+        })
+
+        request(app)
+          .get('/')
+          .expect(500, /Invalid status code/, function (err) {
+            if (isIoJs) {
+              done(err ? null : new Error('expected error'))
+            } else {
+              done(err)
+            }
+          })
+      })
     })
   })
 })