getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
8.02k stars 1.59k forks source link

[v7] Express Regex path middleware breaks URL reconstruction #13884

Open bvanderdrift opened 1 month ago

bvanderdrift commented 1 month ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

7.119.0

Framework Version

Express 4.18.2

Link to Sentry event

https://amplify-30.sentry.io/issues/5962615335/events/c4bd0bfd096743438fae7fd495d33cf8/?project=4506665822584832&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D&referrer=next-event&statsPeriod=7d&stream_index=0

Reproduction Example/SDK Setup

import {init, Integrations, Handlers as SentryHandlers} from '@sentry/node';
import {ProfilingIntegration} from '@sentry/profiling-node';
import express from 'express';

const app = express();

init({
  dsn: process.env.SENTRY_DSN,
  environment: 'development',

  integrations: [
    new Integrations.Http({tracing: true}),
    new Integrations.Express({app}),
    new ProfilingIntegration(),
  ],
});

app.use(SentryHandlers.requestHandler());
app.use(SentryHandlers.tracingHandler());

app.use(
  // /\/(?!foo)/, // ✅ parsed as [GET /debug-sentry/:test]
  // /\/((?!foo).)/, // ❌ parsed as [GET /:0/debug-sentry/:test]
  /\/((?!foo).)*/, // ❌ parsed as [GET /debug-sentry/hell:0/debug-sentry/:test]
  (req, res, next) => {
    next();
  },
);

app.get('/debug-sentry/:test', (req, res) => {
  throw new Error('My first Sentry error!');
});

// The error handler must be registered before any other error middleware and after all controllers
app.use(SentryHandlers.errorHandler());

app.listen(3000);

console.log('Server listening on port 3000');

Steps to Reproduce

Run above code in a single file. Then curl http://localhost:3000/debug-sentry/hello

Expected Result

All are parsed as GET /debug-sentry/:test

Actual Result

Image

AbhiPrasad commented 1 month ago

Hey @bvanderdrift, we probably won't fix this in v7 for now given our other priorities.

Could you check if there is a similar problem in v8?

Will backlog for now!

bvanderdrift commented 1 month ago

@AbhiPrasad understood. We can't use v8 yet in our case because of this unrelated bug: https://github.com/getsentry/sentry-javascript/issues/13871 so don't have an answer there 🤔.