Skip to content

Commit

Permalink
catch rejected promises returned from router.param
Browse files Browse the repository at this point in the history
add test for catching rejected promises

add error middleware test

move isPromise into util folder

update README with router.params promise info

switch to using npm package is-promise
  • Loading branch information
jonchurch committed Apr 19, 2020
1 parent ca507ee commit 7419292
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 15 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ Maps the specified path parameter `name` to a specialized param-capturing middle

This function positions the middleware in the same stack as `.use`.

If a `Promise` object is returned from the `param_middleware` function, the router
will attach an `onRejected` callback using `.then`. If the promise is rejected,
`next` will be called with the rejected value, or an error if the value is falsy.

Parameter mapping is used to provide pre-conditions to routes
which use normalized placeholders. For example a _:user_id_ parameter
could automatically load a user's information from the database without
Expand Down
8 changes: 7 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var mixin = require('utils-merge')
var parseUrl = require('parseurl')
var Route = require('./lib/route')
var setPrototypeOf = require('setprototypeof')
var isPromise = require('is-promise')

/**
* Module variables.
Expand Down Expand Up @@ -634,7 +635,12 @@ function processParams (params, layer, called, req, res, done) {
if (!fn) return param()

try {
fn(req, res, paramCallback, paramVal, key.name)
var ret = fn(req, res, paramCallback, paramVal, key.name)
if (isPromise(ret)) {
ret.then(null, function (error) {
paramCallback(error || new Error('Rejected promise'))
})
}
} catch (e) {
paramCallback(e)
}
Expand Down
15 changes: 1 addition & 14 deletions lib/layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/

var pathRegexp = require('path-to-regexp')
var isPromise = require('is-promise')

/**
* Module variables.
Expand Down Expand Up @@ -187,20 +188,6 @@ function decodeParam (val) {
}
}

/**
* Returns true if the val is a Promise.
*
* @param {*} val
* @return {boolean}
* @private
*/

function isPromise (val) {
return val &&
typeof val === 'object' &&
typeof val.then === 'function'
}

/**
* Loosens the given path for path-to-regexp matching.
*/
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"repository": "pillarjs/router",
"dependencies": {
"array-flatten": "3.0.0",
"is-promise": "^2.1.0",
"methods": "~1.1.2",
"parseurl": "~1.3.3",
"path-to-regexp": "3.2.0",
Expand Down
18 changes: 18 additions & 0 deletions test/param.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,24 @@ describe('Router', function () {
.expect(500, /Error: boom/, done)
})

it('should catch rejected promises returned from fn', function (done) {
var router = new Router()
var server = createServer(router)

router.param('user', function parseUser (req, res, next, user) {
return Promise.reject(new Error('boom'))
})

router.get('/user/:user', function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('get user ' + req.params.id)
})

request(server)
.get('/user/bob')
.expect(500, /Error: boom/, done)
})

describe('next("route")', function () {
it('should cause route with param to be skipped', function (done) {
var cb = after(3, done)
Expand Down
18 changes: 18 additions & 0 deletions test/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,24 @@ describe('Router', function () {
.get('/')
.expect(500, done)
})
it('should invoke error function after router.param returns rejected promise', function (done) {
var router = new Router()
var server = createServer(router)

router.param('user', function (req, res, next, user) {
return Promise.reject(new Error('boom!'))
})

router.get('/:user', function handle (req, res, next) {
res.end()
})

router.use(sawError)

request(server)
.get('/username')
.expect(200, 'saw Error: boom!', done)
})
})

describe('next("route")', function () {
Expand Down

0 comments on commit 7419292

Please sign in to comment.