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

[IlyasShabi]

What does this PR do?

Fixes a bug in Router instrumentation where calling HTTP methods like router.bind('/') without handlers causes unexpected errors due to changed argument passing behavior.

Motivation

The PR #6271 (endpoint collection feature) changed how arguments are passed to Express route methods. This changed Express's behavior when methods are called without handlers and this will impact getting handlers in
router

Given the example router.bind('/') this is how router will register handlers:

original.call(this, fn, ...otherArgs) // handlers are [undefined] - Router validates undefined as a handler and throws

original.apply(this, arguments) // handlers are [] - nothing to do

Edit:

Even with the first solution we still encounter another bug related to router stack which is undefined in some early Express 4 versions (latest one not concerned). We had to fix this by adding optional chaining only when needed

See SCP-950 for more details

[github-actions]

Overall package size

Self size: 13.23 MB
Deduped: 116.13 MB
No deduping: 131.15 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 10.3.0 | 20.73 MB | 20.74 MB | | @datadog/native-iast-taint-tracking | 4.0.0 | 11.72 MB | 11.73 MB | | @datadog/pprof | 5.12.0 | 11.19 MB | 11.57 MB | | @opentelemetry/resources | 1.30.1 | 557.67 kB | 7.71 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.4 | 2.95 MB | 5.82 MB | | @datadog/wasm-js-rewriter | 5.0.1 | 2.82 MB | 3.55 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api-logs | 0.208.0 | 199.48 kB | 1.42 MB | | @opentelemetry/api | 1.9.0 | 1.22 MB | 1.22 MB | | jsonpath-plus | 10.3.0 | 617.18 kB | 1.08 MB | | import-in-the-middle | 1.15.0 | 127.66 kB | 856.24 kB | | lru-cache | 10.4.3 | 804.3 kB | 804.3 kB | | @datadog/openfeature-node-server | 0.1.0-preview.15 | 106.53 kB | 424.55 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | pprof-format | 2.2.1 | 163.06 kB | 163.06 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | @isaacs/ttlcache | 2.1.1 | 90.58 kB | 90.58 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 7.0.5 | 63.38 kB | 63.38 kB | | istanbul-lib-coverage | 3.2.2 | 34.37 kB | 34.37 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.3 | 23.74 kB | 23.74 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | mutexify | 1.4.0 | 5.71 kB | 8.74 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | module-details-from-path | 1.0.4 | 3.96 kB | 3.96 kB | | escape-string-regexp | 5.0.0 | 3.66 kB | 3.66 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

[codecov]

Codecov Report

❌ Patch coverage is 33.33333% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.84%. Comparing base (e5e826e) to head (af29a5a).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
packages/datadog-instrumentations/src/router.js	12.50%	7 Missing ⚠️
packages/datadog-instrumentations/src/express.js	57.14%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6908      +/-   ##
==========================================
- Coverage   83.85%   83.84%   -0.01%     
==========================================
  Files         506      506              
  Lines       21371    21373       +2     
==========================================
+ Hits        17920    17921       +1     
- Misses       3451     3452       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[BridgeAR]

Please add a regression test

[simon-id]

@BridgeAR i was with Ilyas on zoom a minute ago, and his original plan was to add a test, and he tried, but IIUC couldn't figure a way to do so. The edgecase is super weird. Our theory is that the customer is using router.bind(this) thinking he's using the bind method from the function prototype, but in reality router.bind() declares an express route for the BIND http method.
I'll let Ilyas explain why he's struggling to write the test (after his lunch)

[simon-id]

according to the original commit where we made the mistake, it's not the only place where we did: 562c741 (#6271)

1. express.js wrappedAll()
2. express.js wrappedRoute()

[pr-commenter]

Benchmarks

Benchmark execution time: 2025-11-13 18:01:51

Comparing candidate commit af29a5a in PR branch ishabi/router-missing-cb with baseline commit e5e826e in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1602 metrics, 68 unstable metrics.

[IlyasShabi]

@simon-id I double checked wrappedAll and wrappedRoute before creating the PR and both function are same, the only suspect is path which could not be undefined

[simon-id]

@IlyasShabi

@simon-id I double checked wrappedAll and wrappedRoute before creating the PR and both function are same, the only suspect is path which could not be undefined

It doesn't matter if they're not affected, as we discussed with Watson, this is more of a "good practice" thing than an actual bug. We don't want people to copy paste this (arg1, ...args) syntax somewhere else and have the same issue by accident. Does that make sense ? It should almost be a linter rule

[simon-id]

LGTM but as ruben said, I'd also like to have either a test or an explanation of why creating a test is not worth it

[simon-id]

@BridgeAR So turns out Ilyas was really struggling to make his non-reg test pass, simply because there was A SECOND BUG in the instrumentation that have been there for 4 months, so he got completely thrown of course. We debugged together and found the issue (with a lot of difficulties). So we're fixing two bugs in this PR instead of just one. And the non-reg test passes!

we added the test