Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle rejected promises from router.param #92

Merged
merged 1 commit into from
Dec 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

This incorporates all changes after 1.3.5 up to 1.3.6.

* Add support for returned, rejected Promises to `router.param`

2.0.0-beta.1 / 2020-03-29
=========================

Expand Down
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ Maps the specified path parameter `name` to a specialized param-capturing middle

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

The function can optionally return a `Promise` object. If a `Promise` object
is returned from the 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 @@ -13,6 +13,7 @@
*/

var flatten = require('array-flatten').flatten
var isPromise = require('is-promise')
var Layer = require('./lib/layer')
var methods = require('methods')
var mixin = require('utils-merge')
Expand Down Expand Up @@ -642,7 +643,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'))
dougwilson marked this conversation as resolved.
Show resolved Hide resolved
})
}
} 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 @@ -12,6 +12,7 @@
* @private
*/

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

/**
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": "4.0.0",
"methods": "~1.1.2",
"parseurl": "~1.3.3",
"path-to-regexp": "3.2.0",
Expand Down
44 changes: 44 additions & 0 deletions test/param.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ var shouldNotHitHandle = utils.shouldNotHitHandle
var createServer = utils.createServer
var request = utils.request

var describePromises = global.Promise ? describe : describe.skip

describe('Router', function () {
describe('.param(name, fn)', function () {
it('should reject missing name', function () {
Expand Down Expand Up @@ -254,6 +256,48 @@ describe('Router', function () {
.expect(500, /Error: boom/, done)
})

describePromises('promise support', function () {
it('should pass rejected promise value', 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)
})

it('should pass rejected promise without value', function (done) {
var router = new Router()
var server = createServer(router)

router.use(function createError (req, res, next) {
return Promise.reject() // eslint-disable-line prefer-promise-reject-errors
})

router.param('user', function parseUser (req, res, next, user) {
return Promise.reject() // eslint-disable-line prefer-promise-reject-errors
})

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: Rejected promise/, done)
})
})

describe('next("route")', function () {
it('should cause route with param to be skipped', function (done) {
var cb = after(3, done)
Expand Down