fastify
fastify copied to clipboard
fastify
routerPath returns url with a trailing slash
Prerequisites
- [X] I have written a descriptive issue title
- [X] I have searched existing issues to ensure the issue has not already been raised
Issue
Originally this error comes from fastify-metrics plugin. It relies on two things:
-
onRoutehook to set up a metric for the route; - request
routerPathproperty to find the route metric from the request context.
The problem happens when you use a plugin prefix with the prefixTrailingSlash option set to both (default) value. In this case, the onRoute hook will be called only once, so fastify-metrics will set up only one route, but internally there are two different routes with two different routerPath properties. So fastify-metrics will not record metrics for requests with trailing slash.
Example:
'use strict'
const fastify = require('fastify')()
fastify.addHook('onRoute', ({ method, url }) => {
console.log('New route added:', method, url)
})
fastify.register(async (fastify) => {
fastify.get('/', async (request) => {
console.log(request.routerPath)
})
}, { prefix: '/foo' })
fastify.ready(() => {
fastify.inject({ method: 'GET', url: '/foo' })
fastify.inject({ method: 'GET', url: '/foo/' })
})
Output:
New route added: GET /foo
New route added: HEAD /foo
New route added: HEAD /foo/
/foo
/foo/
If we don't call onRoute twice, that means that we want to hide the second route from a user. So from the user's perspective, there should be only one route. If it's true, then we should return routerPath with a trimmed trailing slash.
Maybe we should clone the options like https://github.com/fastify/fastify/pull/4273 for the /foo/ route.
If it's true, then we should return routerPath with a trimmed trailing slash.
Agreed!
Hi @mcollina can I try to work on this isssue ? or someone has already take it ?
The second request in this example should return/foo instead of /foo/.
Expected output:
New route added: GET /foo
New route added: HEAD /foo
New route added: HEAD /foo/ <- not sure that we should call onRoute hook for this route
/foo
/foo <- without trailing slash
The second request in this example should return
/fooinstead of/foo/. Expected output:New route added: GET /foo New route added: HEAD /foo New route added: HEAD /foo/ <- not sure that we should call onRoute hook for this route /foo /foo <- without trailing slash
you can fix it by using :
const fastify = require('fastify')({
ignoreTrailingSlash: true
})
- If one option works wrong, you can't fix it by setting another option. My example without
ignoreTrailingSlashstill would work wrong. - If you add
ignoreTrailingSlash=trueit would trim trailing slashes from all routes, not only from bare prefixes.
'use strict'
const fastify = require('fastify')()
fastify.addHook('onRoute', ({ method, url }) => {
console.log('New route added:', method, url)
})
fastify.register(async (fastify) => {
fastify.get('/', async (request) => {
console.log(request.routerPath)
})
fastify.get('/bar/', async (request) => {
console.log(request.routerPath)
})
}, { prefix: '/foo' })
fastify.ready(() => {
fastify.inject({ method: 'GET', url: '/foo' })
fastify.inject({ method: 'GET', url: '/foo/' })
fastify.inject({ method: 'GET', url: '/foo/bar/' })
})
It shouldn't trim /foo/bar/, only /foo/, because it's a prefix.
I've made some research on the code and made some debuging. but I ask if it's really an issue ? because when you give a URL to a prefix is not right to get the URL with the prefix behind? the routerPath return just the URL passed on the request
The problem is inconsistency. Every time fastify registers a new route, the onRoute hook triggers. So I as a user can handle each new route. So it confuses me as a user when I see in the routerPath param the route that I haven't registered.
the routerPath return just the URL passed on the request
no, it returns a route that was registered, not a URL string
I am interested in contributing to this issue. Please let me know if there is anything else I should be mindful of.
@ivan-tymoshenko I think the code is behaving as it is suppose to. If you would like to get rid of trailing slash from (/foo/) from being registered, instead of registering the route as fastify.get('/'), you could instead do fastify.get('') that would give you the expected result. Let me know if that helps resolve the issue.
