fixup! fixup! fixup! feat(core, node): portable Express integration

Author: isaacs

packages/core/src/integrations/express/patch-layer.ts

@@ -13,7 +13,6 @@ import {
   ATTR_HTTP_ROUTE,
   ATTR_EXPRESS_TYPE,
   ATTR_EXPRESS_NAME,
-  ExpressLayerType_MIDDLEWARE,
   ExpressLayerType_ROUTER,
 } from './types';
 import {
@@ -33,9 +32,6 @@ export type ExpressPatchLayerOptions = Pick<
   'onRouteResolved' | 'ignoreLayers' | 'ignoreLayersType'
 >;
 
-// exported for testing
-export const kDidSetSDKMetadata = Symbol('sentry.DidSetSDKMetadata');
-
 export function patchLayer(options: ExpressPatchLayerOptions, maybeLayer?: ExpressLayer, layerPath?: string): void {
   if (!maybeLayer?.handle) {
     return;
@@ -56,7 +52,7 @@ export function patchLayer(options: ExpressPatchLayerOptions, maybeLayer?: Expre
 
   function layerHandlePatched(
     this: ExpressLayer,
-    req: ExpressRequest & { [kDidSetSDKMetadata]?: boolean },
+    req: ExpressRequest,
     res: ExpressResponse,
     //oxlint-disable-next-line no-explicit-any
     ...otherArgs: any[]
@@ -105,7 +101,7 @@ export function patchLayer(options: ExpressPatchLayerOptions, maybeLayer?: Expre
       // prevent improper layer calculation in the case where there's a
       // middleware without a layerPath argument. It's unclear whether
       // that's possible, or if any existing code depends on that "bug".
-      if (isLayerPathStored && type === ExpressLayerType_MIDDLEWARE) {
+      if (isLayerPathStored) {
         storedLayers.pop();
       }
       return layerHandleOriginal.apply(this, [req, res, ...otherArgs]);

packages/core/test/lib/integrations/express/patch-layer.test.ts

@@ -264,6 +264,34 @@ describe('patchLayer', () => {
     checkSpans([]);
   });
 
+  it('pops storedLayers when ignoring router or request_handler type layers', () => {
+    for (const type of ['router', 'request_handler'] as const) {
+      const options: ExpressPatchLayerOptions = { ignoreLayersType: [type] };
+      const req = Object.assign(new EventEmitter(), {
+        originalUrl: '/a/b/c/layerPath',
+      }) as unknown as ExpressRequest;
+
+      // simulate layers already stored for previous path segments
+      storeLayer(req, '/a');
+      storeLayer(req, '/b');
+
+      // patch a layer of the ignored type with a layerPath
+      const layerHandleOriginal = vi.fn();
+      // layer.name must match what getLayerMetadata uses to classify each type:
+      // 'router' → router, 'bound dispatch' → request_handler, other → middleware
+      const layerName = type === 'router' ? 'router' : 'bound dispatch';
+      const layer = { name: layerName, handle: layerHandleOriginal } as unknown as ExpressLayer;
+      patchLayer(options, layer, '/c');
+
+      // storeLayer('/c') happens inside the patched handle, before being popped
+      // after handle returns, storedLayers should be back to ['/a', '/b']
+      layer.handle(req, Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse);
+
+      // the ignored layer's path must be cleaned up so subsequent layers see the correct route
+      expect(getStoredLayers(req)).toStrictEqual(['/a', '/b']);
+    }
+  });
+
   it('warns about not setting name in default isolation scope', async () => {
     inDefaultIsolationScope = true;
     DEBUG_BUILD = true;