refactor error handling

battbeach · Jul 4, 2012 · f176ea2 · f176ea2
1 parent a82a264
commit f176ea2
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 26 deletions.
diff --git a/lib/send.js b/lib/send.js
@@ -126,26 +126,19 @@ SendStream.prototype.maxage = function(ms){
 };
 
 /**
- * Emit error `status` and `msg`.
+ * Emit error with `status`.
  *
  * @param {Number} status
- * @param {String} msg
  * @api private
  */
 
-SendStream.prototype.error = function(status, msg){
-  var err = new Error(msg);
+SendStream.prototype.error = function(status){
   var res = this.res;
+  var err = new Error(http.STATUS_CODES[status]);
   err.status = status;
-
-  if (this.listeners('error').length) {
-    this.emit('error', err);
-    return;
-  }
-
-  status = status || 500;
-  res.statusCode = status;
-  res.end(http.STATUS_CODES[status]);
+  if (this.listeners('error').length) return this.emit('error', err);
+  res.statusCode = err.status;
+  res.end(err.message);
 };
 
 /**
@@ -244,8 +237,8 @@ SendStream.prototype.isCachable = function(){
 
 SendStream.prototype.onStatError = function(err){
   var notfound = ['ENOENT', 'ENAMETOOLONG', 'ENOTDIR'];
-  if (~notfound.indexOf(err.code)) return this.error(404, 'not found');
-  this.error(500, err);
+  if (~notfound.indexOf(err.code)) return this.error(404);
+  this.error(500);
 };
 
 /**
@@ -295,22 +288,22 @@ SendStream.prototype.pipe = function(res){
 
   // invalid request uri
   path = utils.decode(path);
-  if (-1 == path) return this.error(400, 'invalid request uri');
+  if (-1 == path) return this.error(400);
 
   // null byte(s)
-  if (~path.indexOf('\0')) return this.error(400, 'invalid request uri');
+  if (~path.indexOf('\0')) return this.error(400);
 
   // join / normalize from optional root dir
   if (root) path = normalize(join(this._root, path));
 
   // ".." is malicious without "root"
-  if (this.isMalicious()) return this.error(403, 'forbidden');
+  if (this.isMalicious()) return this.error(403);
 
   // malicious path
-  if (root && 0 != path.indexOf(root)) return this.error(403, 'forbidden');
+  if (root && 0 != path.indexOf(root)) return this.error(403);
 
   // hidden file support
-  if (!this._hidden && this.hasLeadingDot()) return this.error(404, 'not found');
+  if (!this._hidden && this.hasLeadingDot()) return this.error(404);
 
   // index file support
   if (this._index && this.hasTrailingSlash()) path += this._index;
@@ -359,7 +352,7 @@ SendStream.prototype.send = function(path, stat){
     // unsatisfiable
     if (-1 == ranges) {
       res.setHeader('Content-Range', 'bytes */' + stat.size);
-      return this.error(416, 'requested range not satisfiable');
+      return this.error(416);
     }
 
     // valid (syntactically invalid ranges are treated as a regular response)

diff --git a/test/send.js b/test/send.js
@@ -39,7 +39,7 @@ describe('send(file).pipe(res)', function(){
   it('should treat a malformed URI as a bad request', function(done){
     request(app)
     .get('/some%99thing.txt')
-    .expect('invalid request uri', done);
+    .expect('Bad Request', done);
   })
 
   it('should treat an ENAMETOOLONG as a 404', function(done){
@@ -101,7 +101,7 @@ describe('send(file).pipe(res)', function(){
     request(app)
     .get('/meow')
     .expect(404)
-    .expect('not found')
+    .expect('Not Found')
     .end(done);
   })
 
@@ -308,7 +308,7 @@ describe('send(file, options)', function(){
       request(app)
       .get('/.secret')
       .expect(404)
-      .expect('not found')
+      .expect('Not Found')
       .end(done);
     })
   })
@@ -338,7 +338,7 @@ describe('send(file, options)', function(){
 
         request(app)
         .get('/pets/../../send.js')
-        .expect('forbidden')
+        .expect('Forbidden')
         .end(done);
       })
     })
@@ -348,7 +348,7 @@ describe('send(file, options)', function(){
         request(app)
         .get('/../send.js')
         .expect(403)
-        .expect('forbidden')
+        .expect('Forbidden')
         .end(done);
       })
     })