Permalink
Browse files

fix(db): ensure that requests for bad paths are logged

Fixes #144
  • Loading branch information...
1 parent 5edb677 commit dd6d8e2325456f6266134793310343db47ce36f0 @philbooth philbooth committed Jul 2, 2015
Showing with 128 additions and 27 deletions.
  1. +35 −25 index.js
  2. +93 −2 test/backend/remote.js
View
@@ -5,6 +5,7 @@
var restify = require('restify')
var bufferize = require('./lib/bufferize')
var version = require('./package.json').version
+var errors = require('./lib/error')
function createServer(db) {
@@ -30,34 +31,39 @@ function createServer(db) {
res.send(bufferize.unbuffer(result || {}))
}
},
- function (err) {
- if (typeof err !== 'object') {
- err = { message: err || 'none' }
- }
- var statusCode = err.code || 500
- api.emit(
- 'failure',
- {
- code: statusCode,
- route: req.route.name,
- method: req.method,
- path: req.url,
- err: err,
- t: Date.now() - req.time(),
- }
- )
-
- res.send(statusCode, {
- message: err.message,
- errno: err.errno,
- error: err.error,
- code: err.code
- })
- }
+ handleError.bind(null, req, res)
)
.done(next, next)
}
}
+
+ function handleError (req, res, err) {
+ if (typeof err !== 'object') {
+ err = { message: err || 'none' }
+ }
+
+ var statusCode = err.code || 500
+
+ api.emit(
+ 'failure',
+ {
+ code: statusCode,
+ route: req.route ? req.route.name : 'unknown',
+ method: req.method,
+ path: req.url,
+ err: err,
+ t: Date.now() - req.time(),
+ }
+ )
+
+ res.send(statusCode, {
+ message: err.message,
+ errno: err.errno,
+ error: err.error,
+ code: err.code
+ })
+ }
+
var api = restify.createServer()
api.use(restify.bodyParser())
api.use(bufferize.bufferizeRequest)
@@ -114,10 +120,14 @@ function createServer(db) {
}, 15000)
memInterval.unref()
+ api.on('NotFound', function (req, res) {
+ handleError(req, res, errors.notFound())
+ })
+
return api
}
module.exports = {
createServer: createServer,
- errors: require('./lib/error')
+ errors: errors
}
@@ -52,10 +52,21 @@ function testNotFound(t, err) {
}, 'Object contains no other fields')
}
+// Helper function that tests for the server failure event.
+//
+// Takes two arguments:
+//
+// 1. the test object (t)
+// 2. the restify server object (server)
+function captureFailureEvents(t, server) {
+ server.on('failure', t.pass.bind(t, 'The server emitted the failure event'))
+}
+
// To run these tests from a new backend, create a DB instance, start a test server
// and pass the config containing the connection params to this function. The tests
-// will run against that server.
-module.exports = function(cfg) {
+// will run against that server. Second argument is the restify server object, for
+// testing of events via `server.on`.
+module.exports = function(cfg, server) {
var d = P.defer()
@@ -551,6 +562,86 @@ module.exports = function(cfg) {
)
test(
+ 'GET an unknown path',
+ function (t) {
+ t.plan(3)
+ captureFailureEvents(t, server)
+ client.getThen('/foo')
+ .then(function(r) {
+ t.fail('This request should have failed (instead it suceeded)')
+ t.end()
+ }, function(err) {
+ testNotFound(t, err)
+ t.end()
+ })
+ }
+ )
+
+ test(
+ 'PUT an unknown path',
+ function (t) {
+ t.plan(3)
+ captureFailureEvents(t, server)
+ client.putThen('/bar', {})
+ .then(function(r) {
+ t.fail('This request should have failed (instead it suceeded)')
+ t.end()
+ }, function(err) {
+ testNotFound(t, err)
+ t.end()
+ })
+ }
+ )
+
+ test(
+ 'POST an unknown path',
+ function (t) {
+ t.plan(3)
+ captureFailureEvents(t, server)
+ client.postThen('/baz', {})
+ .then(function(r) {
+ t.fail('This request should have failed (instead it suceeded)')
+ t.end()
+ }, function(err) {
+ testNotFound(t, err)
+ t.end()
+ })
+ }
+ )
+
+ test(
+ 'DELETE an unknown path',
+ function (t) {
+ t.plan(3)
+ captureFailureEvents(t, server)
+ client.delThen('/qux')
+ .then(function(r) {
+ t.fail('This request should have failed (instead it suceeded)')
+ t.end()
+ }, function(err) {
+ testNotFound(t, err)
+ t.end()
+ })
+ }
+ )
+
+ test(
+ 'HEAD an unknown path',
+ function (t) {
+ t.plan(2)
+ captureFailureEvents(t, server)
+ client.headThen('/wibble')
+ .then(function(r) {
+ t.fail('This request should have failed (instead it suceeded)')
+ t.end()
+ }, function(err) {
+ t.deepEqual(err.body, {}, 'Body is empty since this is a HEAD request')
+ t.end()
+ })
+ }
+ )
+
+ test(
'teardown',
function (t) {
d.resolve()

0 comments on commit dd6d8e2

Please sign in to comment.