TypeError: partialRoute.split is not a function when using array as path argument

[albanv]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

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

1. use sentry tracing express integration
2. have a route declared with an array as path parameter route.get(['path', 'other_path', /regex_path/], (req, res, next) => {...
3. try to access the route

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

TypeError: partialRoute.split is not a function
  File "/app/node_modules/@sentry/tracing/cjs/integrations/node/express.js", line 220, col 8, in Function.process_params
    .split('/')
  File "/app/node_modules/express/lib/router/index.js", line 280, col 10, in next
    self.process_params(layer, paramcalled, req, res, function (err) {
  File "/app/node_modules/express/lib/router/index.js", line 175, col 3, in Function.handle
    next();
  File "/app/node_modules/express/lib/router/index.js", line 47, col 12, in router
    router.handle(req, res, next);
  File "/app/node_modules/express/lib/router/layer.js", line 95, col 5, in Layer.handle [as handle_request]
    fn(req, res, next);
  File "/app/node_modules/express/lib/router/index.js", line 328, col 13, in trim_prefix
    layer.handle_request(req, res, next);
  File "/app/node_modules/express/lib/router/index.js", line 286, col 9, in <anonymous>
    trim_prefix(layer, layerError, layerPath, path)
  File "/app/node_modules/express/lib/router/index.js", line 346, col 12, in Function.process_params
    return done();
  File "/app/node_modules/@sentry/tracing/cjs/integrations/node/express.js", line 245, col 34, in Function.process_params
    return originalProcessParams.call(this, layer, called, req, res, done);
  File "/app/node_modules/express/lib/router/index.js", line 280, col 10, in next
    self.process_params(layer, paramcalled, req, res, function (err) {
  File "/app/node_modules/express/lib/router/index.js", line 646, col 15, in <anonymous>
    return fn.apply(this, arguments);
  File "/app/node_modules/express/lib/router/index.js", line 265, col 14, in next
    return done(layerError);
  File "/app/node_modules/express/lib/router/index.js", line 175, col 3, in Function.handle
    next();
  File "/app/node_modules/express/lib/router/index.js", line 47, col 12, in router
    router.handle(req, res, next);
  File "/app/node_modules/express/lib/router/layer.js", line 95, col 5, in Layer.handle [as handle_request]
    fn(req, res, next);
  File "/app/node_modules/express/lib/router/index.js", line 328, col 13, in trim_prefix
    layer.handle_request(req, res, next);
  File "/app/node_modules/express/lib/router/index.js", line 286, col 9, in <anonymous>
    trim_prefix(layer, layerError, layerPath, path)
  File "/app/node_modules/express/lib/router/index.js", line 346, col 12, in Function.process_params
    return done();
  File "/app/node_modules/@sentry/tracing/cjs/integrations/node/express.js", line 245, col 34, in Function.process_params
    return originalProcessParams.call(this, layer, called, req, res, done);
  File "/app/node_modules/express/lib/router/index.js", line 280, col 10, in next
    self.process_params(layer, paramcalled, req, res, function (err) {
  File "/app/node_modules/express/lib/router/index.js", line 646, col 15, in <anonymous>
    return fn.apply(this, arguments);
  File "/app/node_modules/express/lib/router/index.js", line 265, col 14, in next
    return done(layerError);
  File "/app/node_modules/express/lib/router/index.js", line 175, col 3, in Function.handle
    next();
  File "/app/node_modules/express/lib/router/index.js", line 47, col 12, in router
    router.handle(req, res, next);
  File "/app/node_modules/express/lib/router/layer.js", line 95, col 5, in Layer.handle [as handle_request]
    fn(req, res, next);
  File "/app/node_modules/express/lib/router/index.js", line 328, col 13, in trim_prefix
    layer.handle_request(req, res, next);
  File "/app/node_modules/express/lib/router/index.js", line 286, col 9, in <anonymous>
    trim_prefix(layer, layerError, layerPath, path)
  File "/app/node_modules/express/lib/router/index.js", line 346, col 12, in Function.process_params
    return done();
    ```

[AbhiPrasad]

Hey, thanks for writing in. We are aware of the issue and will be posting a fix soon. Thanks!

[Lms24]

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)?

[albanv]

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 ?

[Lms24]

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 Layer for each array item (string or regex) and call newLayer.match(orignialLayer.path) to find out if that item matched
• Use path-to-regex which 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)