Repository navigation
TypeError: partialRoute.split is not a function when using array as path argument #5489
Description
Activity
Hey, thanks for writing in. We are aware of the issue and will be posting a fix soon. Thanks!
Hi @albanv, I started looking into this.
Can you confirm that before 7.8.0, for the route array you posted,
route.get(['path', 'other_path', /regex_path/], (req, res, next) => {...})
your transaction names would look like this:
GET /path,/other_path,/regex_path/If yes, is this what you'd expect? Or would you expect to only get the part of the route that was actually matched (e.g.
GET /other_path)?Hi @Lms24, yes, the transaction names looked like what you say.
But now that you ask, I think it should be more useful to only have the part of the route that was matched. In my example, it will ease a lot to find the culprit if there is an issue with the regex for instance.
Nonetheless, in an ideal world I would like to have both, the whole route with the matched part made obvious.
something like: GET /path,/other_path,/regex_path/
What do you think ?Thanks for your feedback @albanv. I agree, the matched part would probably be better to get as a transaction name. However, I think we need more feedback and time to think about this before doing anything in that regard. But we can come back and revisit this. For the moment, I'm just gonna fix the error (see linked PR above).
I don't think we'll be able to provide both (or highlight the matched part as suggested) easily as this requires a lot of changes in different parts and teams. However, we could for example use the matched part as the transaction name and add the whole array to the event context.
Just leaving some more context here in for future readers: To find out which item in the routes array would actually match, we'd need to match each individual item against the incoming path to figure this out. Express internally doesn't care which part matched because upon layer creation it just throws all array items into one regex and matches raw paths against this regex. So we'd have to do the matching ourselves.
There are a few options how to do this:
- Create a
Layerfor each array item (string or regex) and callnewLayer.match(orignialLayer.path)to find out if that item matched - Use
path-to-regexwhich Express uses internally. Convert the items into Regex and test the raw URL against this. - Maybe possible: Try to convert string paths into RegExp manually and do the matching ourselves (i.e. no dependencies required)
Reacted by Alban- Create a
Is there an existing issue for this?
How do you use Sentry?
Sentry Saas (sentry.io)
Which package are you using?
@sentry/tracing
SDK Version
7.8.0
Framework Version
express 4.18.1, node 18.7.0
Link to Sentry event
https://sentry.io/organizations/eatwith/issues/3461507981/
Steps to Reproduce
route.get(['path', 'other_path', /regex_path/], (req, res, next) => {...As discussed in #5481 with @Lms24, in express arrays are also valids path parameters.
#5483 only fix the issue when using RegExp as path parameter.
Expected Result
The route handler should execute its actual logic
Actual Result
Sentry tracing express integration does crash