fix(tracer): make instrumenting hono middlewares more robust (#6250)

The current implementation expected the onError method to always
be present. This is now handled appropriately and fixes the access
to the path, in case it is missing.

Fixes #6230

Author: BridgeAR

packages/datadog-instrumentations/src/hono.js

@@ -17,32 +17,35 @@ function wrapFetch (fetch) {
   }
 }
 
+function onErrorFn (error, _context_) {
+  throw error
+}
+
 function wrapCompose (compose) {
-  return function (middleware, onError, onNotFound) {
+  return function (middlewares, onError, onNotFound) {
+    onError ??= onErrorFn
+
     const instrumentedOnError = (...args) => {
       const [error, context] = args
       const req = context.env.incoming
       errorChannel.publish({ req, error })
       return onError(...args)
     }
 
-    const instrumentedMiddlewares = middleware.map(h => {
+    const instrumentedMiddlewares = middlewares.map(h => {
       const [[fn, meta], params] = h
 
-      // TODO: handle middleware instrumentation
       const instrumentedFn = (...args) => {
         const context = args[0]
-        const req = context.env.incoming
-        const route = meta.path
         routeChannel.publish({
-          req,
-          route
+          req: context.env.incoming,
+          route: meta?.path
         })
         return fn(...args)
       }
       return [[instrumentedFn, meta], params]
     })
-    return compose.apply(this, [instrumentedMiddlewares, instrumentedOnError, onNotFound])
+    return compose.call(this, instrumentedMiddlewares, instrumentedOnError, onNotFound)
   }
 }
 

packages/datadog-plugin-hono/test/integration-test/client.spec.js

@@ -8,15 +8,15 @@ const {
   assertObjectContains,
 } = require('../../../../integration-tests/helpers')
 
-describe('esm', () => {
+describe('esm integration test', () => {
   let agent
   let proc
   let sandbox
 
-  withVersions('hono', 'hono', version => {
+  withVersions('hono', 'hono', (range, _moduleName_, version) => {
     before(async function () {
       this.timeout(50000)
-      sandbox = await createSandbox([`'hono@${version}'`, '@hono/[email protected]'], false,
+      sandbox = await createSandbox([`'hono@${range}'`, '@hono/[email protected]'], false,
         ['./packages/datadog-plugin-hono/test/integration-test/*'])
     })
 
@@ -35,13 +35,25 @@ describe('esm', () => {
     })
 
     it('is instrumented', async () => {
-      proc = await spawnPluginIntegrationTestProc(sandbox.folder, 'server.mjs', agent.port)
+      proc = await spawnPluginIntegrationTestProc(sandbox.folder, 'server.mjs', agent.port, undefined, {
+        VERSION: version
+      })
       proc.url += '/hello'
 
       return curlAndAssertMessage(agent, proc, ({ headers, payload }) => {
         assertObjectContains(headers, { host: `127.0.0.1:${agent.port}` })
-        // TODO: Fix the resource! It should be 'GET /hello'
-        // This seems to be a generic ESM issue, also e.g., on express.
+        assertObjectContains(payload, [[{ name: 'hono.request', resource: 'GET /hello' }]])
+      })
+    }).timeout(50000)
+
+    it('receives missing route trace', async () => {
+      proc = await spawnPluginIntegrationTestProc(sandbox.folder, 'server.mjs', agent.port, undefined, {
+        VERSION: version
+      })
+      proc.url += '/missing'
+
+      return curlAndAssertMessage(agent, proc, ({ headers, payload }) => {
+        assertObjectContains(headers, { host: `127.0.0.1:${agent.port}` })
         assertObjectContains(payload, [[{ name: 'hono.request', resource: 'GET' }]])
       })
     }).timeout(50000)

packages/datadog-plugin-hono/test/integration-test/server.mjs

@@ -2,10 +2,34 @@ import 'dd-trace/init.js'
 import { Hono } from 'hono'
 import { serve } from '@hono/node-server'
 
+import process from 'node:process'
+
 const app = new Hono()
 
+const version = process.env.VERSION.split('.').map(Number)
+
+const hasCombine = version[0] > 4 || version[0] === 4 && version[1] >= 5
+const response = 'green energy\n'
+
+if (hasCombine) {
+  const { every } = await import('hono/combine')
+  app.use(every(async (context, next) => {
+    context.set('response', response)
+    return next()
+  }))
+} else {
+  app.use(async (context, next) => {
+    context.set('response', response)
+    return next()
+  })
+}
+
 app.get('/hello', (c) => {
-  return c.text('green energy\n')
+  const res = c.get('response')
+  if (res !== response) {
+    throw new Error(`Expected response to be "${response}", got "${res}"`)
+  }
+  return c.text(res)
 })
 
 serve({