fix(express): 2 crashes when router[method]() is used with no handler (#6908)

Co-authored-by: simon-id <simon.id@datadoghq.com>

Author: IlyasShabi

packages/datadog-instrumentations/src/express.js

@@ -76,26 +76,28 @@ function wrapResponseRender (render) {
 }
 
 function wrapAppAll (all) {
-  return function wrappedAll (path, ...otherArgs) {
-    if (!routeAddedChannel.hasSubscribers) return all.call(this, path, ...otherArgs)
+  return function wrappedAll (...args) {
+    if (!routeAddedChannel.hasSubscribers) return all.apply(this, args)
 
+    const path = args[0]
     const paths = normalizeRoutePaths(path)
 
     for (const p of paths) {
       routeAddedChannel.publish({ method: '*', path: p })
     }
 
-    return all.call(this, path, ...otherArgs)
+    return all.apply(this, args)
   }
 }
 
 // Wrap app.route() to instrument Route object
 function wrapAppRoute (route) {
-  return function wrappedRoute (path, ...otherArgs) {
-    const routeObj = route.call(this, path, ...otherArgs)
+  return function wrappedRoute (...args) {
+    const routeObj = route.apply(this, args)
 
     if (!routeAddedChannel.hasSubscribers) return routeObj
 
+    const path = args[0]
     const paths = normalizeRoutePaths(path)
 
     if (!paths.length) return routeObj

packages/datadog-instrumentations/src/router.js

@@ -146,33 +146,34 @@ function createWrapRouterMethod (name) {
   }
 
   function wrapMethod (original) {
-    return shimmer.wrapFunction(original, original => function methodWithTrace (fn, ...otherArgs) {
+    return shimmer.wrapFunction(original, original => function methodWithTrace (...args) {
       let offset = 0
       if (this.stack) {
         offset = Array.isArray(this.stack) ? this.stack.length : 1
       }
-      const router = original.call(this, fn, ...otherArgs)
+      const router = original.apply(this, args)
 
       if (typeof this.stack === 'function') {
         this.stack = [{ handle: this.stack }]
       }
 
       if (routeAddedChannel.hasSubscribers) {
-        routeAddedChannel.publish({ topOfStackFunc: methodWithTrace, layer: this.stack.at(-1) })
+        routeAddedChannel.publish({ topOfStackFunc: methodWithTrace, layer: this.stack?.at(-1) })
       }
 
+      const fn = args[0]
+
       // Publish only if this router was mounted by app.use() (prevents early '/sub/...')
       if (routeAddedChannel.hasSubscribers && isAppMounted(this) && this.stack?.length > offset) {
         // Handle nested router mounting for 'use' method
-        if (original.name === 'use' && otherArgs.length >= 1) {
+        if (original.name === 'use' && args.length >= 2) {
           const { mountPaths, startIdx } = extractMountPaths(fn)
 
           if (mountPaths.length) {
             const parentPaths = getRouterMountPaths(this)
-            const callArgs = [fn, ...otherArgs]
 
-            for (let i = startIdx; i < callArgs.length; i++) {
-              const nestedRouter = callArgs[i]
+            for (let i = startIdx; i < args.length; i++) {
+              const nestedRouter = args[i]
 
               if (!nestedRouter || typeof nestedRouter !== 'function') continue
 
@@ -206,7 +207,7 @@ function createWrapRouterMethod (name) {
         }
       }
 
-      if (this.stack.length > offset) {
+      if (this.stack?.length > offset) {
         wrapStack(this.stack.slice(offset), extractMatchers(fn))
       }
 

packages/datadog-instrumentations/test/express.spec.js

@@ -5,21 +5,22 @@ const { expect } = require('chai')
 const dc = require('dc-polyfill')
 const { describe, it, beforeEach, before, after } = require('mocha')
 const sinon = require('sinon')
+const semifies = require('semifies')
 
 const agent = require('../../dd-trace/test/plugins/agent')
 const { withVersions } = require('../../dd-trace/test/setup/mocha')
 
 withVersions('express', 'express', version => {
   describe('express query instrumentation', () => {
     const queryParserReadCh = dc.channel('datadog:query:read:finish')
-    let port, server, requestBody
+    let port, server, requestBody, express
 
     before(() => {
       return agent.load(['express', 'body-parser'], { client: false })
     })
 
     before((done) => {
-      const express = require(`../../../versions/express@${version}`).get()
+      express = require(`../../../versions/express@${version}`).get()
       const app = express()
       app.get('/', (req, res) => {
         requestBody()
@@ -73,5 +74,13 @@ withVersions('express', 'express', version => {
 
       queryParserReadCh.unsubscribe(blockRequest)
     })
+
+    if (semifies(version, '4')) {
+      // Router does not exist in Express 5
+      it('should work correctly when router[method] is called without handler', () => {
+        const router = express.Router()
+        expect(() => { router.bind('/test') }).to.not.throw()
+      })
+    }
   })
 })