From e2e00c5c7f9c0bc9e2ce31bb517b2776091a6dfa Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:57:16 +0200 Subject: [PATCH 1/8] fix(hono): Make the Hono middleware types compile on Hono 5 Hono 5 removes c.req.matchedRoutes and c.req.routePath. The vendored HonoRequest type required both, so app.use(honoMiddleware(app)) failed with TS2769 on Hono 5. Make matchedRoutes optional and drop routePath, which the SDK does not read. Add Vitest type tests against Hono 4 and a pinned Hono 5 release candidate, a type check of the SDK calls in the Hono 4 E2E apps, and a lint rule that keeps hono imports out of @sentry/server-utils. Pin the hono devDependency of @sentry/hono to 4.13.9, so the package no longer builds against the hoisted version by chance. Refs #25223 Co-Authored-By: Claude Opus 5.5 --- .../hono-4-legacy/package.json | 3 +- .../hono-4-legacy/src/sentry.typecheck.ts | 20 +++++++ .../hono-4-legacy/tsconfig.typecheck.json | 6 ++ .../test-applications/hono-4/package.json | 3 +- .../hono-4/src/honoMiddleware.typecheck.ts | 13 ++++ .../hono-4/tsconfig.typecheck.json | 6 ++ packages/hono/package.json | 2 + packages/hono/test/types.test-d.ts | 59 +++++++++++++++++++ packages/hono/vite.config.ts | 8 +++ packages/server-utils/.oxlintrc.json | 10 ++++ .../src/integrations/hono/honoTypes.ts | 6 +- .../src/orchestrion/config/hono.ts | 3 + yarn.lock | 10 ++++ 13 files changed, 143 insertions(+), 6 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/hono-4-legacy/src/sentry.typecheck.ts create mode 100644 dev-packages/e2e-tests/test-applications/hono-4-legacy/tsconfig.typecheck.json create mode 100644 dev-packages/e2e-tests/test-applications/hono-4/src/honoMiddleware.typecheck.ts create mode 100644 dev-packages/e2e-tests/test-applications/hono-4/tsconfig.typecheck.json create mode 100644 packages/hono/test/types.test-d.ts diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/package.json b/dev-packages/e2e-tests/test-applications/hono-4-legacy/package.json index c707ee5f2608..231900020d4f 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4-legacy/package.json +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/package.json @@ -9,7 +9,8 @@ "dev:bun": "bun src/entry.bun.ts", "dev:deno": "deno run --allow-net --allow-env --allow-read src/entry.deno.ts", "build": "wrangler deploy --dry-run", - "test:build": "pnpm install && pnpm build", + "test:build": "pnpm install && pnpm typecheck && pnpm build", + "typecheck": "tsc --noEmit -p tsconfig.typecheck.json", "test:assert": "TEST_ENV=production playwright test" }, "dependencies": { diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/src/sentry.typecheck.ts b/dev-packages/e2e-tests/test-applications/hono-4-legacy/src/sentry.typecheck.ts new file mode 100644 index 000000000000..14237ae092ae --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/src/sentry.typecheck.ts @@ -0,0 +1,20 @@ +// Type check only: `tsc --noEmit` in `test:build` checks that these calls compile with the real Hono +// types. Nothing imports or runs this file. +import { sentry as sentryBun } from '@sentry/hono/bun'; +import { sentry as sentryCloudflare } from '@sentry/hono/cloudflare'; +import { sentry as sentryDeno } from '@sentry/hono/deno'; +import { sentry as sentryNode } from '@sentry/hono/node'; +import { Hono } from 'hono'; + +type AppEnv = { Bindings: { E2E_TEST_DSN: string }; Variables: { user: string } }; + +const dsn = 'https://public@dsn.ingest.sentry.io/1337'; + +const app = new Hono(); +app.use(sentryNode(app)); +app.use(sentryBun(app, { dsn })); +app.use(sentryDeno(app, { dsn })); +app.use(sentryCloudflare(app, env => ({ dsn: env.E2E_TEST_DSN }))); + +const api = app.basePath('/api'); +api.use(sentryNode(api)); diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tsconfig.typecheck.json b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tsconfig.typecheck.json new file mode 100644 index 000000000000..62059f72a594 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tsconfig.typecheck.json @@ -0,0 +1,6 @@ +{ + // Checks only the type-only files that call the SDK with the real Hono types. The rest of the app + // source runs on several runtimes and has no Node, Bun or Deno globals in its types. + "extends": "./tsconfig.json", + "include": ["src/*.typecheck.ts"] +} diff --git a/dev-packages/e2e-tests/test-applications/hono-4/package.json b/dev-packages/e2e-tests/test-applications/hono-4/package.json index 4efa27a21f54..6aba3043d05f 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/package.json +++ b/dev-packages/e2e-tests/test-applications/hono-4/package.json @@ -8,7 +8,8 @@ "dev:bun": "bun run dist/entry.bun.js", "dev:deno": "deno run --allow-net --allow-env --allow-read --preload=npm:@sentry/deno/import src/entry.deno.ts", "dev:cloudflare": "wrangler dev --config ./dist/hono_4/wrangler.json --var \"E2E_TEST_DSN:$E2E_TEST_DSN\" --log-level=$(test $CI && echo 'none' || echo 'log')", - "test:build": "pnpm install", + "test:build": "pnpm install && pnpm typecheck", + "typecheck": "tsc --noEmit -p tsconfig.typecheck.json", "test": "TEST_ENV=production playwright test", "test:assert": "pnpm test:assert:node", "test:assert:node": "RUNTIME=node pnpm test", diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/honoMiddleware.typecheck.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/honoMiddleware.typecheck.ts new file mode 100644 index 000000000000..6083ce500085 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/honoMiddleware.typecheck.ts @@ -0,0 +1,13 @@ +// Type check only: `tsc --noEmit` in `test:build` checks that these calls compile with the real Hono +// types. Nothing imports or runs this file. +import * as Sentry from '@sentry/node'; +import { Hono } from 'hono'; + +type AppEnv = { Bindings: { E2E_TEST_DSN: string }; Variables: { user: string } }; + +const app = new Hono(); +app.use(Sentry.honoMiddleware(app)); +app.use('*', Sentry.honoMiddleware(app)); + +const api = app.basePath('/api'); +api.use(Sentry.honoMiddleware(api)); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tsconfig.typecheck.json b/dev-packages/e2e-tests/test-applications/hono-4/tsconfig.typecheck.json new file mode 100644 index 000000000000..62059f72a594 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/hono-4/tsconfig.typecheck.json @@ -0,0 +1,6 @@ +{ + // Checks only the type-only files that call the SDK with the real Hono types. The rest of the app + // source runs on several runtimes and has no Node, Bun or Deno globals in its types. + "extends": "./tsconfig.json", + "include": ["src/*.typecheck.ts"] +} diff --git a/packages/hono/package.json b/packages/hono/package.json index 982ee76c2748..f3b00ef9b8c1 100644 --- a/packages/hono/package.json +++ b/packages/hono/package.json @@ -110,6 +110,8 @@ "@cloudflare/workers-types": "4.20250922.0", "@hono/node-server": "^2.0.10", "@types/node": "^18.19.1", + "hono": "4.13.9", + "hono-5": "npm:hono@5.0.0-rc.1", "wrangler": "4.62.0" }, "scripts": { diff --git a/packages/hono/test/types.test-d.ts b/packages/hono/test/types.test-d.ts new file mode 100644 index 000000000000..cd2149874fab --- /dev/null +++ b/packages/hono/test/types.test-d.ts @@ -0,0 +1,59 @@ +import { honoMiddleware } from '@sentry/server-utils'; +import { Hono } from 'hono'; +import type { Hono as Hono5 } from 'hono-5'; +import { describe, it } from 'vitest'; +import { sentry as sentryBun } from '../src/bun/middleware'; +import { sentry as sentryCloudflare } from '../src/cloudflare/middleware'; +import { sentry as sentryDeno } from '../src/deno/middleware'; +import { sentry as sentryNode } from '../src/node/middleware'; + +type AppEnv = { Bindings: { SENTRY_DSN: string }; Variables: { user: string } }; + +declare const app5: Hono5; + +const dsn = 'https://public@dsn.ingest.sentry.io/1337'; + +describe('Hono 4', () => { + it('accepts honoMiddleware(app)', () => { + const app = new Hono(); + app.use(honoMiddleware(app)); + + const api = app.basePath('/api'); + api.use(honoMiddleware(api)); + }); + + it('accepts sentry(app) from each runtime entry', () => { + const app = new Hono(); + app.use(sentryNode(app)); + app.use(sentryBun(app, { dsn })); + app.use(sentryDeno(app, { dsn })); + app.use(sentryCloudflare(app, env => ({ dsn: env.SENTRY_DSN }))); + + const api = app.basePath('/api'); + api.use(sentryNode(api)); + }); +}); + +describe('Hono 5', () => { + it('accepts honoMiddleware(app)', () => { + app5.use(honoMiddleware(app5)); + + const api = app5.basePath('/api'); + api.use(honoMiddleware(api)); + }); + + // In this package `hono` resolves to Hono 4, and `Hono` has `private` members, so a Hono 5 app never + // matches the `Hono` type of `sentry()`. In a Hono 5 app, the same import resolves to Hono 5, so a + // Hono 5 app checks these calls with its own types. Each `@ts-expect-error` fails if the call starts + // to compile here. + it('rejects sentry(app) here, because `hono` is Hono 4 in this package', () => { + // @ts-expect-error -- Hono 5 app against the Hono 4 types of `sentry()` + app5.use(sentryNode(app5)); + // @ts-expect-error -- Hono 5 app against the Hono 4 types of `sentry()` + app5.use(sentryBun(app5, { dsn })); + // @ts-expect-error -- Hono 5 app against the Hono 4 types of `sentry()` + app5.use(sentryDeno(app5, { dsn })); + // @ts-expect-error -- Hono 5 app against the Hono 4 types of `sentry()` + app5.use(sentryCloudflare(app5, env => ({ dsn: env.SENTRY_DSN }))); + }); +}); diff --git a/packages/hono/vite.config.ts b/packages/hono/vite.config.ts index b2150cd225a4..bac0e296db14 100644 --- a/packages/hono/vite.config.ts +++ b/packages/hono/vite.config.ts @@ -3,4 +3,12 @@ import baseConfig from '../../vite/vite.config'; export default defineConfig({ ...baseConfig, + test: { + ...baseConfig.test, + typecheck: { + enabled: true, + tsconfig: './tsconfig.test.json', + ignoreSourceErrors: true, + }, + }, }); diff --git a/packages/server-utils/.oxlintrc.json b/packages/server-utils/.oxlintrc.json index d9e07c9dd0cd..001bf4e0b298 100644 --- a/packages/server-utils/.oxlintrc.json +++ b/packages/server-utils/.oxlintrc.json @@ -31,6 +31,16 @@ "name": "diagnostics_channel", "allowTypeImports": true, "message": "Bun garbage-collects a channel that no code references, together with its subscribers. Import the wrappers from `src/utils/diagnosticsChannel.ts` instead. See https://github.com/oven-sh/bun/issues/43086" + }, + { + "name": "hono", + "message": "`@sentry/server-utils` has no `hono` dependency and must work on every Hono major. Use the vendored types in `src/integrations/hono/honoTypes.ts`." + } + ], + "patterns": [ + { + "group": ["hono/*"], + "message": "`@sentry/server-utils` has no `hono` dependency and must work on every Hono major. Use the vendored types in `src/integrations/hono/honoTypes.ts`." } ] } diff --git a/packages/server-utils/src/integrations/hono/honoTypes.ts b/packages/server-utils/src/integrations/hono/honoTypes.ts index a4dc01e14152..9ef1d15b64a6 100644 --- a/packages/server-utils/src/integrations/hono/honoTypes.ts +++ b/packages/server-utils/src/integrations/hono/honoTypes.ts @@ -34,10 +34,8 @@ export interface HonoRequest { method: string; path: string; routeIndex: number; - // These are public (though deprecated) getters on Hono's request. Reading them avoids a runtime - // import of the `hono/route` helpers, which are their non-deprecated replacements. - matchedRoutes: HonoRoute[]; - routePath: string; + // Deprecated getter that Hono 5 removes. + matchedRoutes?: HonoRoute[]; [key: string]: any; } diff --git a/packages/server-utils/src/orchestrion/config/hono.ts b/packages/server-utils/src/orchestrion/config/hono.ts index eda1a8084ff3..74075179e31a 100644 --- a/packages/server-utils/src/orchestrion/config/hono.ts +++ b/packages/server-utils/src/orchestrion/config/hono.ts @@ -8,6 +8,9 @@ const honoInstrumentationConfig: InstrumentationConfig[] = [ { // `new Context()` runs once per request: where the Sentry middleware is injected and matched // middleware handlers are wrapped for spans. + // TODO(v12): Raise the minimum Hono version to 4.4.9, here and in the `hono` peer range of + // `@sentry/hono`. Before 4.4.9, `#dispatch` passes `matchResult` to `new HonoRequest()`, not to + // `new Context()`, so this hook gets no match result and instruments nothing. channelName: 'context', module: { name: 'hono', versionRange: '>=4.0.0 <5', filePath: /^dist\/(?:cjs\/)?context\.js$/ }, functionQuery: { className: 'Context' }, diff --git a/yarn.lock b/yarn.lock index ef8f10db812c..493b5df4dbc4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17184,6 +17184,16 @@ homedir-polyfill@^1.0.1: dependencies: parse-passwd "^1.0.0" +"hono-5@npm:hono@5.0.0-rc.1": + version "5.0.0-rc.1" + resolved "https://sfw.security.sentry.io/npm/hono/-/hono-5.0.0-rc.1.tgz#7115abe957dd0a5b794e999267dec3a6b217196f" + integrity sha512-vTcfU9yAggXh/+ZSi8zpYbzYDubCgFnSt8d5afCAlgkd38dyYb3CVtOh8YWhS65NkEqBseCsTj/P8mceJ+7KNA== + +hono@4.13.9: + version "4.13.9" + resolved "https://sfw.security.sentry.io/npm/hono/-/hono-4.13.9.tgz#682943308ab2f228012460d596a9af58090a1528" + integrity sha512-7dMkQmZoC4E6F7AtaQSPhlWAdnBti+j7rreMZl8QB4jFiEhP9TWbGWUMi8WYzBCgmgulxuvLQupKqo+Co6Omyg== + hono@^4.13.7: version "4.13.7" resolved "https://registry.yarnpkg.com/hono/-/hono-4.13.7.tgz#80fdab8326f03d02f049117fd968101933493600" From a93c25c95ac8824a099d50fc3839f22a2fd44164 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 16 Oct 2026 14:28:03 +0200 Subject: [PATCH 2/8] build: Deduplicate hono in yarn.lock The hono devDependency of @sentry/hono (4.13.9) and the integration tests (^4.13.7) resolved to two Hono copies. Their HonoRequest types differ by a unique symbol, so sentry(app) did not type-check in the bun and cloudflare integration tests, and yarn dedupe-deps:check failed. Resolve both to 4.13.9. Refs #25223 Co-Authored-By: Claude Opus 5.5 --- yarn.lock | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/yarn.lock b/yarn.lock index 493b5df4dbc4..a01932a72162 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17189,16 +17189,11 @@ homedir-polyfill@^1.0.1: resolved "https://sfw.security.sentry.io/npm/hono/-/hono-5.0.0-rc.1.tgz#7115abe957dd0a5b794e999267dec3a6b217196f" integrity sha512-vTcfU9yAggXh/+ZSi8zpYbzYDubCgFnSt8d5afCAlgkd38dyYb3CVtOh8YWhS65NkEqBseCsTj/P8mceJ+7KNA== -hono@4.13.9: +hono@4.13.9, hono@^4.13.7: version "4.13.9" resolved "https://sfw.security.sentry.io/npm/hono/-/hono-4.13.9.tgz#682943308ab2f228012460d596a9af58090a1528" integrity sha512-7dMkQmZoC4E6F7AtaQSPhlWAdnBti+j7rreMZl8QB4jFiEhP9TWbGWUMi8WYzBCgmgulxuvLQupKqo+Co6Omyg== -hono@^4.13.7: - version "4.13.7" - resolved "https://registry.yarnpkg.com/hono/-/hono-4.13.7.tgz#80fdab8326f03d02f049117fd968101933493600" - integrity sha512-c8/gF9ac8Y78/agExVocyLevgR+JlpNB444Py0FSX8pJoPdYUfUzRcXtYEYGwt6l19qIlVZPN5Mfsw9jFShmQQ== - hookable@^5.5.3: version "5.5.3" resolved "https://registry.yarnpkg.com/hookable/-/hookable-5.5.3.tgz#6cfc358984a1ef991e2518cb9ed4a778bbd3215d" From 2a39959cefe0223f4bb85fb86bd13ef6285d9573 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 16 Oct 2026 14:41:26 +0200 Subject: [PATCH 3/8] ref(hono): Shorten comments Co-Authored-By: Claude Opus 5.5 --- .../hono-4-legacy/src/sentry.typecheck.ts | 3 +-- .../hono-4-legacy/tsconfig.typecheck.json | 3 +-- .../hono-4/src/honoMiddleware.typecheck.ts | 3 +-- .../hono-4/tsconfig.typecheck.json | 3 +-- packages/hono/test/types.test-d.ts | 14 ++++++-------- .../server-utils/src/orchestrion/config/hono.ts | 5 ++--- 6 files changed, 12 insertions(+), 19 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/src/sentry.typecheck.ts b/dev-packages/e2e-tests/test-applications/hono-4-legacy/src/sentry.typecheck.ts index 14237ae092ae..9df86b4b4b85 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4-legacy/src/sentry.typecheck.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/src/sentry.typecheck.ts @@ -1,5 +1,4 @@ -// Type check only: `tsc --noEmit` in `test:build` checks that these calls compile with the real Hono -// types. Nothing imports or runs this file. +// Only `tsc` in `test:build` reads this file. Nothing runs it. import { sentry as sentryBun } from '@sentry/hono/bun'; import { sentry as sentryCloudflare } from '@sentry/hono/cloudflare'; import { sentry as sentryDeno } from '@sentry/hono/deno'; diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tsconfig.typecheck.json b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tsconfig.typecheck.json index 62059f72a594..9131a4be8ad3 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tsconfig.typecheck.json +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tsconfig.typecheck.json @@ -1,6 +1,5 @@ { - // Checks only the type-only files that call the SDK with the real Hono types. The rest of the app - // source runs on several runtimes and has no Node, Bun or Deno globals in its types. + // The rest of `src` needs Node, Bun and Deno types, which this app does not install. "extends": "./tsconfig.json", "include": ["src/*.typecheck.ts"] } diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/honoMiddleware.typecheck.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/honoMiddleware.typecheck.ts index 6083ce500085..396d90942d11 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/src/honoMiddleware.typecheck.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/honoMiddleware.typecheck.ts @@ -1,5 +1,4 @@ -// Type check only: `tsc --noEmit` in `test:build` checks that these calls compile with the real Hono -// types. Nothing imports or runs this file. +// Only `tsc` in `test:build` reads this file. Nothing runs it. import * as Sentry from '@sentry/node'; import { Hono } from 'hono'; diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tsconfig.typecheck.json b/dev-packages/e2e-tests/test-applications/hono-4/tsconfig.typecheck.json index 62059f72a594..9131a4be8ad3 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tsconfig.typecheck.json +++ b/dev-packages/e2e-tests/test-applications/hono-4/tsconfig.typecheck.json @@ -1,6 +1,5 @@ { - // Checks only the type-only files that call the SDK with the real Hono types. The rest of the app - // source runs on several runtimes and has no Node, Bun or Deno globals in its types. + // The rest of `src` needs Node, Bun and Deno types, which this app does not install. "extends": "./tsconfig.json", "include": ["src/*.typecheck.ts"] } diff --git a/packages/hono/test/types.test-d.ts b/packages/hono/test/types.test-d.ts index cd2149874fab..3d069bd46138 100644 --- a/packages/hono/test/types.test-d.ts +++ b/packages/hono/test/types.test-d.ts @@ -42,18 +42,16 @@ describe('Hono 5', () => { api.use(honoMiddleware(api)); }); - // In this package `hono` resolves to Hono 4, and `Hono` has `private` members, so a Hono 5 app never - // matches the `Hono` type of `sentry()`. In a Hono 5 app, the same import resolves to Hono 5, so a - // Hono 5 app checks these calls with its own types. Each `@ts-expect-error` fails if the call starts - // to compile here. + // In this package, `sentry()` gets its `Hono` type from Hono 4. That class has private members, so a + // Hono 5 app cannot match it. In a real Hono 5 app, `sentry()` gets the Hono 5 types. it('rejects sentry(app) here, because `hono` is Hono 4 in this package', () => { - // @ts-expect-error -- Hono 5 app against the Hono 4 types of `sentry()` + // @ts-expect-error -- Hono 5 app, Hono 4 types app5.use(sentryNode(app5)); - // @ts-expect-error -- Hono 5 app against the Hono 4 types of `sentry()` + // @ts-expect-error -- Hono 5 app, Hono 4 types app5.use(sentryBun(app5, { dsn })); - // @ts-expect-error -- Hono 5 app against the Hono 4 types of `sentry()` + // @ts-expect-error -- Hono 5 app, Hono 4 types app5.use(sentryDeno(app5, { dsn })); - // @ts-expect-error -- Hono 5 app against the Hono 4 types of `sentry()` + // @ts-expect-error -- Hono 5 app, Hono 4 types app5.use(sentryCloudflare(app5, env => ({ dsn: env.SENTRY_DSN }))); }); }); diff --git a/packages/server-utils/src/orchestrion/config/hono.ts b/packages/server-utils/src/orchestrion/config/hono.ts index 74075179e31a..f56ff5a12039 100644 --- a/packages/server-utils/src/orchestrion/config/hono.ts +++ b/packages/server-utils/src/orchestrion/config/hono.ts @@ -8,9 +8,8 @@ const honoInstrumentationConfig: InstrumentationConfig[] = [ { // `new Context()` runs once per request: where the Sentry middleware is injected and matched // middleware handlers are wrapped for spans. - // TODO(v12): Raise the minimum Hono version to 4.4.9, here and in the `hono` peer range of - // `@sentry/hono`. Before 4.4.9, `#dispatch` passes `matchResult` to `new HonoRequest()`, not to - // `new Context()`, so this hook gets no match result and instruments nothing. + // TODO(v12): Raise the minimum Hono version to 4.4.9, here and in the `@sentry/hono` peer range. + // Before 4.4.9, Hono passes `matchResult` to `new HonoRequest()`, so this hook does nothing. channelName: 'context', module: { name: 'hono', versionRange: '>=4.0.0 <5', filePath: /^dist\/(?:cjs\/)?context\.js$/ }, functionQuery: { className: 'Context' }, From 2980a0855f6cde184b37d43e77c94d10762ab96e Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:58:51 +0200 Subject: [PATCH 4/8] ref(hono): Share the middleware classification The Context hook and wrapSubAppMiddleware each had their own copy of the rule that tells middleware from route handlers. Move the rule to getMiddlewareFlags() and check the shape of the match result entries before the hook reads them. No behavior change. Refs #25223 Co-Authored-By: Claude Opus 5.5 --- .../src/integrations/hono/honoIntegration.ts | 55 ++++++------------- .../src/integrations/hono/honoTypes.ts | 4 ++ .../src/integrations/hono/isMiddleware.ts | 20 ++++++- .../src/integrations/hono/matchResult.ts | 26 +++++++++ .../src/integrations/hono/patchRoute.ts | 26 ++------- 5 files changed, 69 insertions(+), 62 deletions(-) create mode 100644 packages/server-utils/src/integrations/hono/matchResult.ts diff --git a/packages/server-utils/src/integrations/hono/honoIntegration.ts b/packages/server-utils/src/integrations/hono/honoIntegration.ts index c2b504db2dab..96091a61c4c0 100644 --- a/packages/server-utils/src/integrations/hono/honoIntegration.ts +++ b/packages/server-utils/src/integrations/hono/honoIntegration.ts @@ -5,7 +5,7 @@ import { HTTP_SERVER } from '@sentry/conventions/op'; import type { IntegrationFn } from '@sentry/core'; import { debug, defineIntegration, startInactiveSpan } from '@sentry/core'; import { DEBUG_BUILD } from '../../debug-build'; -import type { Env, GetConnInfo, Hono, MiddlewareHandler } from './honoTypes'; +import type { Env, GetConnInfo, Hono, MatchedHandler, MiddlewareHandler } from './honoTypes'; import type { OrchestrionChannelContext } from '../../orchestrion/types'; import { CHANNELS } from '../../orchestrion/channels'; import { honoModuleNames } from '../../orchestrion/config/hono'; @@ -14,7 +14,8 @@ import { bindTracingChannelToSpan, safeChannelCallback } from '../../tracing-cha import { applyPatches } from './applyPatches'; import { INTERNAL_REQUEST_ORIGIN } from './constants'; import { createHonoRequestMiddleware } from './createHonoMiddleware'; -import { isMiddleware } from './isMiddleware'; +import { getMiddlewareFlags } from './isMiddleware'; +import { isMatchedHandlerList } from './matchResult'; import { extractPathname, isInternalRequestSpanActive } from './patchAppRequest'; import { wrapMiddlewareWithSpan } from './wrapMiddlewareSpan'; import type { SentryHonoMiddlewareOptions } from './types'; @@ -83,10 +84,14 @@ export function honoMiddleware(app: Hono, options: HonoIntegra }); } -// A Hono `matchResult[0]` entry: `[[handler, routeMeta], paramIndexMap]` — `compose` runs the -// handler (`entry[0][0]`) and the `matchedRoutes` getter reads `routeMeta` (`entry[0][1]`). -// oxlint-disable-next-line typescript/no-explicit-any -type MatchedHandlerEntry = [[any, any], any]; +function wrapMatchedMiddleware(handlers: MatchedHandler[]): void { + const middlewareFlags = getMiddlewareFlags(handlers.map(([[, route]]) => route)); + for (const [i, [pair]] of handlers.entries()) { + if (middlewareFlags[i]) { + pair[0] = wrapMiddlewareWithSpan(pair[0]); + } + } +} /** * Per-request Context hook. `#dispatch` builds `new Context(req, { matchResult })` before its @@ -103,47 +108,19 @@ function injectHonoInstrumentation( requestMiddleware: MiddlewareHandler, injectedHandlerLists: WeakSet, ): void { - const ctorOptions = message.arguments?.[1] as { matchResult?: [MatchedHandlerEntry[], unknown] } | undefined; + const ctorOptions = message.arguments?.[1] as { matchResult?: [unknown[], unknown] } | undefined; const handlers = ctorOptions?.matchResult?.[0]; if (!Array.isArray(handlers) || injectedHandlerLists.has(handlers)) { return; } injectedHandlerLists.add(handlers); - // Classify matched handlers with the same positional heuristic as `wrapSubAppMiddleware`: within a - // method+path group the last handler is the route handler and earlier ones are middleware; `.use()` - // registers as method 'ALL' with a lone genuine-middleware entry, so fall back to arity there. - // Position is needed because a route handler declared with an unused `next` param has middleware arity. - const lastIndexByKey = new Map(); - for (const [i, entry] of handlers.entries()) { - const routeMeta = entry?.[0]?.[1] as { method?: string; path?: string } | undefined; - if (routeMeta?.method != null && routeMeta.path != null) { - // \0 is a collision-free delimiter: it cannot appear in a valid HTTP method or URL path. - lastIndexByKey.set(`${routeMeta.method}\0${routeMeta.path}`, i); - } - } - - for (const [i, entry] of handlers.entries()) { - const pair = entry?.[0]; - const handler = pair?.[0]; - if (typeof handler !== 'function') { - continue; - } - - const routeMeta = pair?.[1] as { method?: string; path?: string } | undefined; - const isMW = - routeMeta?.method != null && routeMeta.path != null - ? lastIndexByKey.get(`${routeMeta.method}\0${routeMeta.path}`) !== i || - (routeMeta.method === 'ALL' && isMiddleware(handler)) - : isMiddleware(handler); - - if (isMW) { - pair[0] = wrapMiddlewareWithSpan(handler as MiddlewareHandler); - } + if (isMatchedHandlerList(handlers)) { + wrapMatchedMiddleware(handlers); } - // Prepend the Sentry request/response middleware. `routeMeta` is what the `matchedRoutes` getter - // exposes; its middleware arity keeps route-name resolution from picking it. + // Prepend the Sentry request/response middleware. `c.req.matchedRoutes` and `hono/route` read + // `routeMeta`; its middleware arity keeps route-name resolution from picking it. const routeMeta = { basePath: '/', path: '/*', method: 'ALL', handler: requestMiddleware }; handlers.unshift([[requestMiddleware, routeMeta], {}]); } diff --git a/packages/server-utils/src/integrations/hono/honoTypes.ts b/packages/server-utils/src/integrations/hono/honoTypes.ts index 9ef1d15b64a6..acd070cd3fe0 100644 --- a/packages/server-utils/src/integrations/hono/honoTypes.ts +++ b/packages/server-utils/src/integrations/hono/honoTypes.ts @@ -29,6 +29,10 @@ export interface HonoRoute { handler: (...args: any[]) => any; } +// An entry of Hono's match result (`matchResult[0]`). `compose` runs `entry[0][0]`; `hono/route` and +// `c.req.matchedRoutes` read `entry[0][1]`. +export type MatchedHandler = [[handler: HonoRoute['handler'], route: HonoRoute], paramIndexMap: unknown]; + export interface HonoRequest { raw: Request; method: string; diff --git a/packages/server-utils/src/integrations/hono/isMiddleware.ts b/packages/server-utils/src/integrations/hono/isMiddleware.ts index 9c18e43e8602..c0ca0c399908 100644 --- a/packages/server-utils/src/integrations/hono/isMiddleware.ts +++ b/packages/server-utils/src/integrations/hono/isMiddleware.ts @@ -1,3 +1,5 @@ +import type { HonoRoute } from './honoTypes'; + // Hono stores the original (unwrapped) handler under this key when it wraps a sub-app's handlers // for a custom `onError`. Unwrapping it lets us inspect the real handler instead of the wrapper. // See https://github.com/honojs/hono/blob/9f0dadf141a3242a6c3b77462c7d33c6ce0f599d/src/hono-base.ts#L224-L226 @@ -12,7 +14,7 @@ const COMPOSED_HANDLER = '__COMPOSED_HANDLER'; * https://github.com/honojs/hono/blob/97c6fe1f12298c715eb7b2da65b4b6e0d81682bb/src/utils/handler.ts#L8 * * This is only one signal — callers that must classify inline middleware sharing a method+path with - * their handler (e.g. `wrapSubAppMiddleware`) additionally need positional information. + * their handler additionally need positional information (`getMiddlewareFlags`). */ export function isMiddleware(handler: unknown): boolean { if (typeof handler !== 'function') { @@ -24,3 +26,19 @@ export function isMiddleware(handler: unknown): boolean { return (original as (...args: unknown[]) => unknown).length >= 2; } + +/** + * Tells for each route whether it is a middleware. Hono has no such flag, so within a method+path + * group the last handler is the route handler and earlier ones are middleware (`app.get('/path', mw, + * handler)`). `.use()` registers as method 'ALL' with a lone middleware entry, so arity decides there. + * Position is needed because a route handler declared with an unused `next` param has middleware arity. + */ +export function getMiddlewareFlags(routes: HonoRoute[]): boolean[] { + // \0 is a collision-free delimiter: it cannot appear in a valid HTTP method or URL path. + const groupKey = (route: HonoRoute): string => `${route.method}\0${route.path}`; + const lastIndexByKey = new Map(routes.map((route, i) => [groupKey(route), i])); + + return routes.map( + (route, i) => lastIndexByKey.get(groupKey(route)) !== i || (route.method === 'ALL' && isMiddleware(route.handler)), + ); +} diff --git a/packages/server-utils/src/integrations/hono/matchResult.ts b/packages/server-utils/src/integrations/hono/matchResult.ts new file mode 100644 index 000000000000..c271a3a7032b --- /dev/null +++ b/packages/server-utils/src/integrations/hono/matchResult.ts @@ -0,0 +1,26 @@ +import { isPlainObject } from '@sentry/core'; +import type { MatchedHandler } from './honoTypes'; + +function isMatchedHandler(entry: unknown): entry is MatchedHandler { + const pair: unknown = Array.isArray(entry) ? entry[0] : undefined; + if (!Array.isArray(pair)) { + return false; + } + + const [handler, route]: unknown[] = pair; + return ( + typeof handler === 'function' && + isPlainObject(route) && + typeof route.method === 'string' && + typeof route.path === 'string' && + typeof route.handler === 'function' + ); +} + +/** + * Checks the shape that `compose` and `hono/route` read from `matchResult[0]`. These are Hono + * internals, so the SDK does not read or wrap the entries of a list of another shape. + */ +export function isMatchedHandlerList(handlers: unknown): handlers is MatchedHandler[] { + return Array.isArray(handlers) && handlers.every(isMatchedHandler); +} diff --git a/packages/server-utils/src/integrations/hono/patchRoute.ts b/packages/server-utils/src/integrations/hono/patchRoute.ts index 4f13b1b760ad..3ee20927b0bd 100644 --- a/packages/server-utils/src/integrations/hono/patchRoute.ts +++ b/packages/server-utils/src/integrations/hono/patchRoute.ts @@ -2,7 +2,7 @@ import { debug, getOriginalFunction } from '@sentry/core'; import type { WrappedFunction } from '@sentry/core'; import { DEBUG_BUILD } from '../../debug-build'; import type { Hono, HonoRoute } from './honoTypes'; -import { isMiddleware } from './isMiddleware'; +import { getMiddlewareFlags } from './isMiddleware'; import { patchAppRequest } from './patchAppRequest'; import { wrapMiddlewareWithSpan } from './wrapMiddlewareSpan'; @@ -104,30 +104,12 @@ export function installRouteHookOnPrototype(honoBaseProto: HonoBaseProto): Route /** * Identifies middleware handlers in a sub-app's flat routes array and wraps them in spans. - * - * Heuristics (since Hono has no "isMiddleware" flag): - * 1. Position: `app.get('/path', mw, handler)` produces entries with the same method+path. - * All but the LAST are middleware (they call `next()`). - * 2. Arity (# of params) for method 'ALL': `.use()` handlers always have 2+ params (context, next), - * while `.all()` route handlers typically have 1 (`context` only). - * See: https://github.com/honojs/hono/blob/18fe604c8cefc2628240651b1af219692e1918c1/src/hono-base.ts#L156-L168 + * See: https://github.com/honojs/hono/blob/18fe604c8cefc2628240651b1af219692e1918c1/src/hono-base.ts#L156-L168 */ export function wrapSubAppMiddleware(routes: HonoRoute[]): void { - const lastIndexByKey = new Map(); - for (const [i, route] of routes.entries()) { - // \0 (null byte) is a collision-free delimiter: it cannot appear in a valid HTTP method name or URL path - lastIndexByKey.set(`${route.method}\0${route.path}`, i); - } - + const middlewareFlags = getMiddlewareFlags(routes); for (const [i, route] of routes.entries()) { - if (typeof route.handler !== 'function') { - continue; - } - - const isLastForGroup = lastIndexByKey.get(`${route.method}\0${route.path}`) === i; - - const isMW = !isLastForGroup || (route.method === 'ALL' && isMiddleware(route.handler)); - if (isMW) { + if (middlewareFlags[i] && typeof route.handler === 'function') { route.handler = wrapMiddlewareWithSpan(route.handler); } } From 1de41db68c0e0a7b82b0ed29b35114a4e1562da9 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 16 Oct 2026 14:42:02 +0200 Subject: [PATCH 5/8] ref(hono): Shorten comments Co-Authored-By: Claude Opus 5.5 --- .../src/integrations/hono/honoIntegration.ts | 4 ++-- .../server-utils/src/integrations/hono/honoTypes.ts | 3 +-- .../server-utils/src/integrations/hono/isMiddleware.ts | 10 +++++----- .../server-utils/src/integrations/hono/matchResult.ts | 4 ++-- .../server-utils/src/integrations/hono/patchRoute.ts | 3 +-- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/server-utils/src/integrations/hono/honoIntegration.ts b/packages/server-utils/src/integrations/hono/honoIntegration.ts index 96091a61c4c0..0ed8a6ac3a7b 100644 --- a/packages/server-utils/src/integrations/hono/honoIntegration.ts +++ b/packages/server-utils/src/integrations/hono/honoIntegration.ts @@ -119,8 +119,8 @@ function injectHonoInstrumentation( wrapMatchedMiddleware(handlers); } - // Prepend the Sentry request/response middleware. `c.req.matchedRoutes` and `hono/route` read - // `routeMeta`; its middleware arity keeps route-name resolution from picking it. + // Prepend the Sentry request/response middleware. Its arity marks it as a middleware, so its `/*` + // never becomes the route name. const routeMeta = { basePath: '/', path: '/*', method: 'ALL', handler: requestMiddleware }; handlers.unshift([[requestMiddleware, routeMeta], {}]); } diff --git a/packages/server-utils/src/integrations/hono/honoTypes.ts b/packages/server-utils/src/integrations/hono/honoTypes.ts index acd070cd3fe0..6ace7bb8f595 100644 --- a/packages/server-utils/src/integrations/hono/honoTypes.ts +++ b/packages/server-utils/src/integrations/hono/honoTypes.ts @@ -29,8 +29,7 @@ export interface HonoRoute { handler: (...args: any[]) => any; } -// An entry of Hono's match result (`matchResult[0]`). `compose` runs `entry[0][0]`; `hono/route` and -// `c.req.matchedRoutes` read `entry[0][1]`. +// One entry of Hono's `matchResult[0]`. export type MatchedHandler = [[handler: HonoRoute['handler'], route: HonoRoute], paramIndexMap: unknown]; export interface HonoRequest { diff --git a/packages/server-utils/src/integrations/hono/isMiddleware.ts b/packages/server-utils/src/integrations/hono/isMiddleware.ts index c0ca0c399908..ee7ad8d08afa 100644 --- a/packages/server-utils/src/integrations/hono/isMiddleware.ts +++ b/packages/server-utils/src/integrations/hono/isMiddleware.ts @@ -28,13 +28,13 @@ export function isMiddleware(handler: unknown): boolean { } /** - * Tells for each route whether it is a middleware. Hono has no such flag, so within a method+path - * group the last handler is the route handler and earlier ones are middleware (`app.get('/path', mw, - * handler)`). `.use()` registers as method 'ALL' with a lone middleware entry, so arity decides there. - * Position is needed because a route handler declared with an unused `next` param has middleware arity. + * Tells for each route if it is a middleware. Hono has no flag for this. In a method+path group, the + * last handler is the route handler (`app.get('/path', mw, handler)`). `.use()` adds middleware with + * method `ALL` and no route handler, so for `ALL` routes the arity also counts. Arity alone is not + * enough, because a route handler can declare an unused `next`. */ export function getMiddlewareFlags(routes: HonoRoute[]): boolean[] { - // \0 is a collision-free delimiter: it cannot appear in a valid HTTP method or URL path. + // `\0` cannot appear in an HTTP method or a URL path. const groupKey = (route: HonoRoute): string => `${route.method}\0${route.path}`; const lastIndexByKey = new Map(routes.map((route, i) => [groupKey(route), i])); diff --git a/packages/server-utils/src/integrations/hono/matchResult.ts b/packages/server-utils/src/integrations/hono/matchResult.ts index c271a3a7032b..e6bd298f1206 100644 --- a/packages/server-utils/src/integrations/hono/matchResult.ts +++ b/packages/server-utils/src/integrations/hono/matchResult.ts @@ -18,8 +18,8 @@ function isMatchedHandler(entry: unknown): entry is MatchedHandler { } /** - * Checks the shape that `compose` and `hono/route` read from `matchResult[0]`. These are Hono - * internals, so the SDK does not read or wrap the entries of a list of another shape. + * Checks the entry shape that `compose` and `hono/route` read. The match result is a Hono internal, so + * the SDK leaves other shapes alone. */ export function isMatchedHandlerList(handlers: unknown): handlers is MatchedHandler[] { return Array.isArray(handlers) && handlers.every(isMatchedHandler); diff --git a/packages/server-utils/src/integrations/hono/patchRoute.ts b/packages/server-utils/src/integrations/hono/patchRoute.ts index 3ee20927b0bd..398df72fb924 100644 --- a/packages/server-utils/src/integrations/hono/patchRoute.ts +++ b/packages/server-utils/src/integrations/hono/patchRoute.ts @@ -103,8 +103,7 @@ export function installRouteHookOnPrototype(honoBaseProto: HonoBaseProto): Route } /** - * Identifies middleware handlers in a sub-app's flat routes array and wraps them in spans. - * See: https://github.com/honojs/hono/blob/18fe604c8cefc2628240651b1af219692e1918c1/src/hono-base.ts#L156-L168 + * Wraps the middleware handlers of a sub-app's routes in spans. */ export function wrapSubAppMiddleware(routes: HonoRoute[]): void { const middlewareFlags = getMiddlewareFlags(routes); From a23bb7a36116ec38be7fee8fe1c554fc75a082ee Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:59:03 +0200 Subject: [PATCH 6/8] fix(hono): Name routes without c.req.matchedRoutes Hono 5 removes c.req.matchedRoutes, so every request was named "GET ". It also restores c.req.routeIndex after next(), so the Sentry middleware can no longer see which handler responded. Read the match result through the getter that hono/route uses, and let each route handler record its routeIndex when it starts. A request with no matched route keeps the runtime's span name. On Hono 4 the names stay the same, except where the old rule picked a later catch-all, for example for a route handler declared with an unused next parameter. Refs #25223 Co-Authored-By: Claude Opus 5.5 --- .../suites/hono-sdk/scenario.mjs | 38 +++++ .../suites/hono-sdk/test.ts | 49 ++++++ .../suites/hono/scenario-honoMiddleware.mjs | 30 ++++ .../suites/hono/scenario.mjs | 22 +++ .../suites/hono/test.ts | 120 +++++++++----- .../utils/runner/createRunner.ts | 4 +- .../test/shared/middlewareHandlers.test.ts | 18 ++- .../hono/test/shared/resolveRouteName.test.ts | 146 ++++++++++++++++++ .../src/integrations/hono/honoIntegration.ts | 6 +- .../src/integrations/hono/matchResult.ts | 82 +++++++++- .../integrations/hono/middlewareHandlers.ts | 14 +- .../src/integrations/hono/resolveRouteName.ts | 42 ++--- 12 files changed, 506 insertions(+), 65 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs create mode 100644 packages/hono/test/shared/resolveRouteName.test.ts diff --git a/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs b/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs index d27dc20bbc30..595d9546c52c 100644 --- a/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs @@ -20,6 +20,44 @@ app.get('/error/:param', () => { throw new Error('Test error from Hono app'); }); +// Route-name cases. `/users/me` and `/users/:id` overlap; the request is named after the handler that +// responded, not after the last one that matched. +app.get('/users/me', c => c.text('me')); +app.get('/users/:id', c => c.text(`user ${c.req.param('id')}`)); + +// A middleware that returns early: no route handler runs, so the name falls back to the last matched +// route handler. +app.use('/early/*', async (c, _next) => c.text('blocked by middleware')); +app.get('/early/:id', c => c.text('never reached')); + +// A middleware-only path: no route handler matches at all. +app.use('/mw-only/*', async (_c, _next) => new Response('handled by middleware')); + +const subApp = new Hono(); +subApp.get('/hello', c => c.text('sub hello')); +app.route('/sub', subApp); + +app.basePath('/base').get('/hello', c => c.text('base hello')); + +app.all('/any-method', c => c.text(`any method: ${c.req.method}`)); + +// Hono answers `HEAD` with the `GET` route. The SDK records no server span for `HEAD`, so the route +// name is checked on the error event. +app.get('/head-error/:id', () => { + throw new Error('Test error from a HEAD request'); +}); + +const innerApp = new Hono(); +innerApp.get('/item/:itemId', c => c.json({ itemId: c.req.param('itemId') })); +app.get('/outer/:itemId', async c => { + const res = await innerApp.request(`/item/${c.req.param('itemId')}`); + return c.json({ outer: c.req.param('itemId'), inner: await res.json() }); +}); + +// A catch-all registered last. It also matches `/users/me` and `/users/:id`, but must not take over +// their names. +app.get('/users/*', c => c.text('users catch-all')); + serve({ fetch: app.fetch, port: 0 }, info => { sendPortToRunner(info.port); }); diff --git a/dev-packages/node-integration-tests/suites/hono-sdk/test.ts b/dev-packages/node-integration-tests/suites/hono-sdk/test.ts index 1366be964a95..160ec4c81892 100644 --- a/dev-packages/node-integration-tests/suites/hono-sdk/test.ts +++ b/dev-packages/node-integration-tests/suites/hono-sdk/test.ts @@ -1,12 +1,61 @@ import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../utils/runner'; +const ROUTE_NAME_CASES: Array<{ method: 'get' | 'post'; path: string; name: string; route: string }> = [ + { method: 'get', path: '/users/me', name: 'GET /users/me', route: '/users/me' }, + { method: 'get', path: '/users/42', name: 'GET /users/:id', route: '/users/:id' }, + { method: 'get', path: '/users/42/posts', name: 'GET /users/*', route: '/users/*' }, + { method: 'get', path: '/early/1', name: 'GET /early/:id', route: '/early/:id' }, + { method: 'get', path: '/mw-only/anything', name: 'GET /mw-only/*', route: '/mw-only/*' }, + { method: 'get', path: '/sub/hello', name: 'GET /sub/hello', route: '/sub/hello' }, + { method: 'get', path: '/base/hello', name: 'GET /base/hello', route: '/base/hello' }, + { method: 'post', path: '/any-method', name: 'POST /any-method', route: '/any-method' }, + { method: 'get', path: '/outer/self-watering-plant', name: 'GET /outer/:itemId', route: '/outer/:itemId' }, +]; + describe('hono-sdk (Node)', () => { afterAll(() => { cleanupChildProcesses(); }); createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + // All cases in one test, because each `test()` starts its own Node process. + test('names each request after the route of the handler that responded', async () => { + const runner = createRunner().unordered(); + for (const { name, route } of ROUTE_NAME_CASES) { + runner.expect({ + span: container => { + const segment = container.items.find(item => item.is_segment && item.name === name); + if (!segment) { + throw new Error(`segment for \`${name}\` not in this container`); + } + expect(segment.attributes?.['http.route']).toEqual({ type: 'string', value: route }); + expect(segment.attributes?.['sentry.segment.name.source']).toEqual({ type: 'string', value: 'route' }); + }, + }); + } + + const started = runner.start(); + for (const { method, path } of ROUTE_NAME_CASES) { + await started.makeRequest(method, path); + } + await started.completed(); + }); + + test('names a HEAD request after the GET route', async () => { + const runner = createRunner() + .ignore('span') + .expect({ + event: { + exception: { values: [{ value: 'Test error from a HEAD request' }] }, + transaction: 'HEAD /head-error/:id', + }, + }) + .start(); + runner.makeRequest('head', '/head-error/1', { expectError: true }); + await runner.completed(); + }); + test('creates a segment span for a basic GET request', async () => { const runner = createRunner() .expect({ diff --git a/dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs b/dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs new file mode 100644 index 000000000000..4e2f624b3319 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs @@ -0,0 +1,30 @@ +import { serve } from '@hono/node-server'; +import * as Sentry from '@sentry/node'; +import { sendPortToRunner } from '@sentry-internal/node-integration-tests'; +import { Hono } from 'hono'; + +// The auto-instrumentation and the manual `honoMiddleware(app)` are both active. +const app = new Hono(); +app.use(Sentry.honoMiddleware(app)); + +// Counts the Sentry wrappers around the running handler. Two SDK paths wrap route handlers here, and +// the router hands both the same cached handler list on each request, so a missing guard would nest one +// more wrapper per request. The match result is read through the only symbol-keyed getter on the +// request prototype (the one that `hono/route` uses). +function wrapperCount(c) { + const [matchResultKey] = Object.getOwnPropertySymbols(Object.getPrototypeOf(c.req)); + let handler = c.req[matchResultKey][0][c.req.routeIndex][0][0]; + let count = 0; + while (handler.__sentry_original__) { + handler = handler.__sentry_original__; + count++; + } + return count; +} + +app.get('/users/me', c => c.json({ wrappers: wrapperCount(c) })); +app.get('/users/:id', c => c.json({ wrappers: wrapperCount(c) })); + +serve({ fetch: app.fetch, port: 0 }, info => { + sendPortToRunner(info.port); +}); diff --git a/dev-packages/node-integration-tests/suites/hono/scenario.mjs b/dev-packages/node-integration-tests/suites/hono/scenario.mjs index ed0f816c03cf..4beb7f97f72d 100644 --- a/dev-packages/node-integration-tests/suites/hono/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/hono/scenario.mjs @@ -97,6 +97,28 @@ app }) .get('/hello', c => c.json({ greeting: 'world' })); +// Route-name cases. `/users/me` and `/users/:id` overlap; the request is named after the handler that +// responded, not after the last one that matched. +app.get('/users/me', c => c.text('me')); +app.get('/users/:id', c => c.text(`user ${c.req.param('id')}`)); + +// A middleware that returns early: no route handler runs, so the name falls back to the last matched +// route handler. +app.use('/early/*', async (c, _next) => c.text('blocked by middleware')); +app.get('/early/:id', c => c.text('never reached')); + +app.all('/any-method', c => c.text(`any method: ${c.req.method}`)); + +// Hono answers `HEAD` with the `GET` route. The SDK records no server span for `HEAD`, so the route +// name is checked on the error event. +app.get('/head-error/:id', () => { + throw new Error('Test error from a HEAD request'); +}); + +// A catch-all registered last. It also matches `/users/me` and `/users/:id`, but must not take over +// their names. +app.get('/users/*', c => c.text('users catch-all')); + serve({ fetch: app.fetch, port: 0 }, info => { sendPortToRunner(info.port); }); diff --git a/dev-packages/node-integration-tests/suites/hono/test.ts b/dev-packages/node-integration-tests/suites/hono/test.ts index 20a16f7e1eba..7fb2dd540d85 100644 --- a/dev-packages/node-integration-tests/suites/hono/test.ts +++ b/dev-packages/node-integration-tests/suites/hono/test.ts @@ -15,12 +15,73 @@ type StreamedSpan = { name?: string; status?: string; is_segment?: boolean; attr const attr = (span: StreamedSpan, key: string): unknown => span.attributes?.[key]?.value; const op = (span: StreamedSpan): unknown => attr(span, 'sentry.op'); +const ROUTE_NAME_CASES: Array<{ method: 'get' | 'post'; path: string; name: string; route: string }> = [ + { method: 'get', path: '/users/me', name: 'GET /users/me', route: '/users/me' }, + { method: 'get', path: '/users/42', name: 'GET /users/:id', route: '/users/:id' }, + { method: 'get', path: '/users/42/posts', name: 'GET /users/*', route: '/users/*' }, + { method: 'get', path: '/overlap/123', name: 'GET /overlap/:id', route: '/overlap/:id' }, + { method: 'get', path: '/early/1', name: 'GET /early/:id', route: '/early/:id' }, + { method: 'get', path: '/mw-only/anything', name: 'GET /mw-only/*', route: '/mw-only/*' }, + { method: 'get', path: '/sub/hello', name: 'GET /sub/hello', route: '/sub/hello' }, + { + method: 'get', + path: '/test-basepath-mw/hello', + name: 'GET /test-basepath-mw/hello', + route: '/test-basepath-mw/hello', + }, + { method: 'post', path: '/any-method', name: 'POST /any-method', route: '/any-method' }, + { method: 'get', path: '/outer/self-watering-plant', name: 'GET /outer/:itemId', route: '/outer/:itemId' }, +]; + +// Matches the span container whose segment has this name, and checks its route. +function segmentNamed(name: string, route: string): { span: (container: { items: StreamedSpan[] }) => void } { + return { + span: container => { + const segment = container.items.find(item => item.is_segment && item.name === name); + if (!segment) { + throw new Error(`segment for \`${name}\` not in this container`); + } + expect(op(segment)).toBe('http.server'); + expect(attr(segment, 'http.route')).toBe(route); + expect(attr(segment, 'sentry.segment.name.source')).toBe('route'); + }, + }; +} + describe('hono auto-instrumentation', () => { afterAll(() => { cleanupChildProcesses(); }); createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + // All cases in one test, because each `test()` starts its own Node process. + test('names each request after the route of the handler that responded', async () => { + const runner = createRunner().unordered(); + for (const { name, route } of ROUTE_NAME_CASES) { + runner.expect(segmentNamed(name, route)); + } + + const started = runner.start(); + for (const { method, path } of ROUTE_NAME_CASES) { + await started.makeRequest(method, path); + } + await started.completed(); + }); + + test('names a HEAD request after the GET route', async () => { + const runner = createRunner() + .unordered() + .expect({ + event: { + exception: { values: [{ value: 'Test error from a HEAD request' }] }, + transaction: 'HEAD /head-error/:id', + }, + }) + .start(); + runner.makeRequest('head', '/head-error/1', { expectError: true }); + await runner.completed(); + }); + test('creates a transaction for a basic GET request', async () => { const runner = createRunner() .unordered() @@ -227,42 +288,6 @@ describe('hono auto-instrumentation', () => { await runner.completed(); }); - test('resolves overlapping handlers to the responding route, not the catch-all', async () => { - const runner = createRunner() - .unordered() - .expect({ - span: container => { - const segment = container.items.find(item => item.is_segment && item.name === 'GET /overlap/:id'); - if (!segment) { - throw new Error('segment for `GET /overlap/:id` not in this container'); - } - // The catch-all `GET /overlap/*` handler also matched, but the parameterized handler - // responded — the route name must not collapse to `/overlap/*`. - expect(attr(segment, 'sentry.segment.name.source')).toBe('route'); - }, - }) - .start(); - runner.makeRequest('get', '/overlap/123'); - await runner.completed(); - }); - - test('falls back to the matched middleware path when only middleware matched', async () => { - const runner = createRunner() - .unordered() - .expect({ - span: container => { - const segment = container.items.find(item => item.is_segment && item.name === 'GET /mw-only/*'); - if (!segment) { - throw new Error('segment for `GET /mw-only/*` not in this container'); - } - expect(op(segment)).toBe('http.server'); - }, - }) - .start(); - runner.makeRequest('get', '/mw-only/anything'); - await runner.completed(); - }); - test('does not set the middleware span to error for a 3xx middleware error', async () => { const runner = createRunner() .unordered() @@ -321,3 +346,26 @@ describe('hono auto-instrumentation', () => { }); }); }); + +describe('hono auto-instrumentation with honoMiddleware(app)', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + createEsmAndCjsTests(__dirname, 'scenario-honoMiddleware.mjs', 'instrument.mjs', (createRunner, test) => { + test('wraps each route handler once and names the request after it', async () => { + const runner = createRunner() + .unordered() + .expect(segmentNamed('GET /users/me', '/users/me')) + .expect(segmentNamed('GET /users/me', '/users/me')) + .expect(segmentNamed('GET /users/:id', '/users/:id')) + .start(); + + // The router returns the same cached handler list for each request to a route. + expect(await runner.makeRequest('get', '/users/me')).toEqual({ wrappers: 1 }); + expect(await runner.makeRequest('get', '/users/me')).toEqual({ wrappers: 1 }); + expect(await runner.makeRequest('get', '/users/42')).toEqual({ wrappers: 1 }); + await runner.completed(); + }); + }); +}); diff --git a/dev-packages/node-integration-tests/utils/runner/createRunner.ts b/dev-packages/node-integration-tests/utils/runner/createRunner.ts index d9fd1d5f149b..edf0c076a6de 100644 --- a/dev-packages/node-integration-tests/utils/runner/createRunner.ts +++ b/dev-packages/node-integration-tests/utils/runner/createRunner.ts @@ -103,7 +103,7 @@ type StartResult = { getPort(): number | undefined; sendSignal(signal: NodeJS.Signals): void; makeRequest( - method: 'get' | 'post' | 'put' | 'delete' | 'patch', + method: 'get' | 'head' | 'post' | 'put' | 'delete' | 'patch', path: string, options?: { headers?: Record; data?: BodyInit; expectError?: boolean; timeout?: number }, ): Promise; @@ -775,7 +775,7 @@ export function createRunner(...paths: string[]) { child?.kill(signal); }, makeRequest: async function ( - method: 'get' | 'post' | 'put' | 'delete' | 'patch', + method: 'get' | 'head' | 'post' | 'put' | 'delete' | 'patch', path: string, options: { headers?: Record; diff --git a/packages/hono/test/shared/middlewareHandlers.test.ts b/packages/hono/test/shared/middlewareHandlers.test.ts index 9253cf56cbd6..b8e3f51c0b67 100644 --- a/packages/hono/test/shared/middlewareHandlers.test.ts +++ b/packages/hono/test/shared/middlewareHandlers.test.ts @@ -47,13 +47,17 @@ const getActiveSpanMock = SentryCore.getActiveSpan as ReturnType; // `event` throws on access so `hasFetchEvent` reports `false` (the Node/Bun/Deno path), which makes // the request handler read request data from `req.raw` rather than a Cloudflare fetch event. -function createMockContext(status: number, error?: Error): unknown { +function createMockContext( + status: number, + error?: Error, + matchedRoutes: unknown[] = [{ basePath: '/', path: '/test', method: 'GET', handler: (_c: unknown) => undefined }], +): unknown { return { req: { method: 'GET', routeIndex: 0, // `resolveRouteName` reads the matched routes straight off the request. - matchedRoutes: [{ basePath: '/', path: '/test', method: 'GET', handler: (_c: unknown) => undefined }], + matchedRoutes, raw: new Request('http://localhost/test'), }, get event(): never { @@ -201,6 +205,16 @@ describe('responseHandler (via createHonoRequestMiddleware)', () => { expect(mockRootSpan.setAttribute).toHaveBeenCalledWith(HTTP_ROUTE, '/test'); expect(mockRootSpan.setAttribute).toHaveBeenCalledWith(SENTRY_SEGMENT_NAME_SOURCE, 'route'); }); + + it('keeps the runtime name when no route matched', async () => { + getActiveSpanMock.mockReturnValue(mockRootSpan); + + await runMiddleware(createMockContext(404, undefined, [])); + + expect(mockRootSpan.updateName).not.toHaveBeenCalled(); + expect(mockRootSpan.setAttribute).not.toHaveBeenCalledWith(HTTP_ROUTE, expect.anything()); + expect(mockSetTransactionName).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/hono/test/shared/resolveRouteName.test.ts b/packages/hono/test/shared/resolveRouteName.test.ts new file mode 100644 index 000000000000..367719eff027 --- /dev/null +++ b/packages/hono/test/shared/resolveRouteName.test.ts @@ -0,0 +1,146 @@ +import * as SentryCore from '@sentry/core'; +import { applyHonoPatches, createHonoRequestMiddleware } from '@sentry/server-utils'; +import { Hono } from 'hono'; +import { HonoRequest } from 'hono/request'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockSetTransactionName = vi.fn(); + +vi.mock('@sentry/core', async () => { + const actual = await vi.importActual('@sentry/core'); + return { + ...actual, + getActiveSpan: vi.fn(() => undefined), + getIsolationScope: vi.fn(() => ({ + setTransactionName: mockSetTransactionName, + setSDKProcessingMetadata: vi.fn(), + })), + }; +}); + +const getIsolationScopeMock = SentryCore.getIsolationScope as ReturnType; + +// Each request gets its own isolation scope, so the middleware's per-request dedup marker never leaks +// into the next request. +function freshIsolationScope(): void { + getIsolationScopeMock.mockReturnValue({ + setTransactionName: mockSetTransactionName, + setSDKProcessingMetadata: vi.fn(), + }); +} + +function createApp({ restoreRouteIndex }: { restoreRouteIndex: boolean }): Hono { + const app = new Hono(); + applyHonoPatches(app); + app.use(createHonoRequestMiddleware()); + + if (restoreRouteIndex) { + // Hono 5's `compose` restores `routeIndex` after `next()` (honojs/hono#4424), so the Sentry + // middleware no longer sees the index of the handler that responded. + app.use(async (c, next) => { + const routeIndex = c.req.routeIndex; + await next(); + c.req.routeIndex = routeIndex; + }); + } + + app.use('/users/*', async (_c, next) => { + await next(); + }); + // A route handler registered with `.use()`, which also gives it a middleware span wrapper. + app.use('/users/via-use', async c => c.text('via use')); + app.get('/users/me', c => c.text('me')); + app.get('/users/:id', c => c.text('user')); + app.use('/mw-only/*', async (_c, _next) => new Response('handled by middleware')); + + const subApp = new Hono(); + subApp.get('/items/:itemId', c => c.text('item')); + app.route('/sub', subApp); + + // A catch-all registered last also matches `/users/me` and `/users/42`, but never responds to them. + app.get('/users/*', c => c.text('catch-all')); + return app; +} + +// Changes a getter of the real Hono 4 request, so that the request looks like one of another Hono. +// Returns the function that restores it. +function overrideRequestProperty(key: PropertyKey, descriptor: PropertyDescriptor): () => void { + const original = Object.getOwnPropertyDescriptor(HonoRequest.prototype, key); + Object.defineProperty(HonoRequest.prototype, key, { configurable: true, ...descriptor }); + return () => { + if (original) { + Object.defineProperty(HonoRequest.prototype, key, original); + } + }; +} + +const hideRequestProperty = (key: PropertyKey): (() => void) => overrideRequestProperty(key, { value: undefined }); + +// The key of the match result getter that `hono/route` uses (an unregistered `Symbol()`). +const [matchResultKey] = Object.getOwnPropertySymbols(HonoRequest.prototype); +if (!matchResultKey) { + throw new Error('The match result getter is missing on `HonoRequest.prototype`.'); +} + +const SOURCES: Array<{ source: string; hidden: PropertyKey[]; restoreRouteIndex: boolean }> = [ + { source: '`c.req.matchedRoutes` (Hono 4)', hidden: [], restoreRouteIndex: false }, + // Hono 5 has no `c.req.matchedRoutes`, so the SDK reads the match result getter. + { source: 'the match result getter (Hono 5)', hidden: ['matchedRoutes'], restoreRouteIndex: true }, +]; + +const CASES: Array<[path: string, expected: string]> = [ + ['/users/me', 'GET /users/me'], + ['/users/42', 'GET /users/:id'], + ['/users/via-use', 'GET /users/via-use'], + ['/users/all/the/things', 'GET /users/*'], + ['/sub/items/7', 'GET /sub/items/:itemId'], + ['/mw-only/anything', 'GET /mw-only/*'], +]; + +describe.each(SOURCES)('route name from $source', ({ hidden, restoreRouteIndex }) => { + let restores: Array<() => void> = []; + + beforeEach(() => { + vi.clearAllMocks(); + restores = hidden.map(hideRequestProperty); + }); + + afterEach(() => { + restores.forEach(restore => restore()); + }); + + it.each(CASES)('names %s as %s', async (path, expected) => { + freshIsolationScope(); + const app = createApp({ restoreRouteIndex }); + + await app.request(path); + + expect(mockSetTransactionName).toHaveBeenLastCalledWith(expected); + }); +}); + +describe('route name from a match result of an unknown shape', () => { + let restores: Array<() => void> = []; + + beforeEach(() => { + vi.clearAllMocks(); + restores = [ + hideRequestProperty('matchedRoutes'), + overrideRequestProperty(matchResultKey, { get: () => [[[null]], {}] }), + ]; + }); + + afterEach(() => { + restores.forEach(restore => restore()); + }); + + it('keeps the request working and does not name it', async () => { + freshIsolationScope(); + const app = createApp({ restoreRouteIndex: true }); + + const response = await app.request('/users/me'); + + expect(response.status).toBe(200); + expect(mockSetTransactionName).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/server-utils/src/integrations/hono/honoIntegration.ts b/packages/server-utils/src/integrations/hono/honoIntegration.ts index 0ed8a6ac3a7b..653ac895f456 100644 --- a/packages/server-utils/src/integrations/hono/honoIntegration.ts +++ b/packages/server-utils/src/integrations/hono/honoIntegration.ts @@ -15,7 +15,7 @@ import { applyPatches } from './applyPatches'; import { INTERNAL_REQUEST_ORIGIN } from './constants'; import { createHonoRequestMiddleware } from './createHonoMiddleware'; import { getMiddlewareFlags } from './isMiddleware'; -import { isMatchedHandlerList } from './matchResult'; +import { isMatchedHandlerList, wrapRouteHandlers } from './matchResult'; import { extractPathname, isInternalRequestSpanActive } from './patchAppRequest'; import { wrapMiddlewareWithSpan } from './wrapMiddlewareSpan'; import type { SentryHonoMiddlewareOptions } from './types'; @@ -97,7 +97,8 @@ function wrapMatchedMiddleware(handlers: MatchedHandler[]): void { * Per-request Context hook. `#dispatch` builds `new Context(req, { matchResult })` before its * single-handler fast-path check, so from the live `matchResult` we: * 1. wrap the matched middleware handlers for spans (route handlers are covered by the request span); - * 2. prepend the Sentry request/response middleware — it drives route naming, request data and error + * 2. wrap the matched route handlers to record which one responded, for the route name; + * 3. prepend the Sentry request/response middleware — it drives route naming, request data and error * capture from inside the chain, and forces the ≥2-handler `compose` path so the fast-path is * never taken. * @@ -117,6 +118,7 @@ function injectHonoInstrumentation( if (isMatchedHandlerList(handlers)) { wrapMatchedMiddleware(handlers); + wrapRouteHandlers(handlers); } // Prepend the Sentry request/response middleware. Its arity marks it as a middleware, so its `/*` diff --git a/packages/server-utils/src/integrations/hono/matchResult.ts b/packages/server-utils/src/integrations/hono/matchResult.ts index e6bd298f1206..4a6982ccf640 100644 --- a/packages/server-utils/src/integrations/hono/matchResult.ts +++ b/packages/server-utils/src/integrations/hono/matchResult.ts @@ -1,5 +1,16 @@ -import { isPlainObject } from '@sentry/core'; -import type { MatchedHandler } from './honoTypes'; +import { addNonEnumerableProperty, isPlainObject } from '@sentry/core'; +import type { Context, HonoRoute, MatchedHandler } from './honoTypes'; +import { getMiddlewareFlags } from './isMiddleware'; + +// The `routeIndex` of the last route handler that started for a request, stored on its `Context`. +// `Symbol.for` so that every copy of `@sentry/server-utils` reads the index that another copy recorded. +const ROUTE_HANDLER_INDEX = Symbol.for('sentry.hono.routeHandlerIndex'); + +// Marks the wrapper that records `ROUTE_HANDLER_INDEX`. The router returns the same cached match +// result for each request to a route, so this keeps every copy of `@sentry/server-utils` from wrapping +// a handler twice. A span wrapper does not count: a route handler that `app.use()` wrapped for a span +// must still record its index. +const RECORDS_ROUTE_HANDLER_INDEX = Symbol.for('sentry.hono.recordsRouteHandlerIndex'); function isMatchedHandler(entry: unknown): entry is MatchedHandler { const pair: unknown = Array.isArray(entry) ? entry[0] : undefined; @@ -24,3 +35,70 @@ function isMatchedHandler(entry: unknown): entry is MatchedHandler { export function isMatchedHandlerList(handlers: unknown): handlers is MatchedHandler[] { return Array.isArray(handlers) && handlers.every(isMatchedHandler); } + +/** + * Reads the request's match result through the getter that `hono/route` uses (Hono 4.8+ and Hono 5). + * Its key is an unregistered `Symbol()` that `hono` does not export, so the getter is found by its + * shape: it is the only symbol-keyed getter on the `HonoRequest` prototype. + */ +export function getMatchedHandlers(context: Context): MatchedHandler[] | undefined { + const requestProto: object | null = Object.getPrototypeOf(context.req); + if (!requestProto) { + return undefined; + } + + const [getterKey, ...otherGetterKeys] = Object.getOwnPropertySymbols(requestProto).filter( + key => typeof Object.getOwnPropertyDescriptor(requestProto, key)?.get === 'function', + ); + if (!getterKey || otherGetterKeys.length > 0) { + return undefined; + } + + const matchResult: unknown = Reflect.get(context.req, getterKey); + return Array.isArray(matchResult) && isMatchedHandlerList(matchResult[0]) ? matchResult[0] : undefined; +} + +/** + * Wraps the route handlers of a match result, so that each records `c.req.routeIndex` when it starts. + * Inside a route handler the index is correct on every Hono major, but Hono 5 restores it after + * `next()`, so the Sentry middleware cannot read it there. + * + * `compose` reads each handler from the match result when it calls it, so a wrap from inside the + * Sentry middleware still covers the handlers that run after it. + */ +export function wrapRouteHandlers(handlers: MatchedHandler[]): void { + const middlewareFlags = getMiddlewareFlags(handlers.map(([[, route]]) => route)); + for (const [i, [pair]] of handlers.entries()) { + if (!middlewareFlags[i]) { + pair[0] = recordRouteHandlerIndex(pair[0]); + } + } +} + +function recordRouteHandlerIndex(handler: HonoRoute['handler']): HonoRoute['handler'] { + if (Reflect.get(handler, RECORDS_ROUTE_HANDLER_INDEX)) { + return handler; + } + + return new Proxy(handler, { + apply(target, thisArg, args: [Context, ...unknown[]]) { + const [context] = args; + addNonEnumerableProperty(context, ROUTE_HANDLER_INDEX, context.req.routeIndex); + return Reflect.apply(target, thisArg, args); + }, + get(target, prop, receiver) { + if (prop === RECORDS_ROUTE_HANDLER_INDEX) { + return true; + } + if (prop === '__sentry_original__') { + return handler; + } + return Reflect.get(target, prop, receiver); + }, + }); +} + +export function getRecordedRouteHandlerIndex(context: Context): number | undefined { + const index: unknown = Reflect.get(context, ROUTE_HANDLER_INDEX); + return typeof index === 'number' ? index : undefined; +} diff --git a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts index d72d357b4c86..f6f208b2b792 100644 --- a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts +++ b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts @@ -13,6 +13,7 @@ import { import type { Context, GetConnInfo } from './honoTypes'; import { hasFetchEvent } from './hono-context'; import { defaultShouldHandleError } from './defaultShouldHandleError'; +import { getMatchedHandlers, wrapRouteHandlers } from './matchResult'; import { resolveRouteName } from './resolveRouteName'; import { type SentryHonoMiddlewareOptions } from './types'; import { HTTP_ROUTE } from '@sentry/conventions/attributes'; @@ -23,6 +24,12 @@ import { HTTP_ROUTE } from '@sentry/conventions/attributes'; export function requestHandler(context: Context, getConnInfo?: GetConnInfo): void { const isolationScope = getCurrentIsolationScope(); + // Without the Context hook (manual setup), this is the only place that sees the match result. + const matchedHandlers = getMatchedHandlers(context); + if (matchedHandlers) { + wrapRouteHandlers(matchedHandlers); + } + // Set a provisional route name as early as possible so events captured during the request already carry the route. // It is re-resolved in `responseHandler` once the middleware chain has run. updateSpanRouteName(isolationScope, context); @@ -87,7 +94,7 @@ export function responseHandler( context: Context, shouldHandleError?: SentryHonoMiddlewareOptions['shouldHandleError'], ): void { - // Overwrite the route name now that the middleware chain has run: `routeIndex` is accurate here + // Re-resolve now that the middleware chain has run and the responding route handler, if any, has recorded its index updateSpanRouteName(getCurrentIsolationScope(), context); captureContextError(context, shouldHandleError); @@ -116,6 +123,11 @@ export function captureContextError( function updateSpanRouteName(isolationScope: Scope, context: Context): void { const route = resolveRouteName(context); + // Keep the runtime's name rather than `GET `. + if (route === undefined) { + return; + } + const routeName = `${context.req.method} ${route}`; const activeSpan = getActiveSpan(); diff --git a/packages/server-utils/src/integrations/hono/resolveRouteName.ts b/packages/server-utils/src/integrations/hono/resolveRouteName.ts index 97b1283722ed..396b6e985131 100644 --- a/packages/server-utils/src/integrations/hono/resolveRouteName.ts +++ b/packages/server-utils/src/integrations/hono/resolveRouteName.ts @@ -1,5 +1,6 @@ import type { Context, HonoRoute } from './honoTypes'; import { isMiddleware } from './isMiddleware'; +import { getMatchedHandlers, getRecordedRouteHandlerIndex } from './matchResult'; // Arity alone is enough here (unlike `wrapSubAppMiddleware` in patchRoute.ts, which also needs position) // We only want the path, and inline middleware shares its handler's path. @@ -7,36 +8,37 @@ function isRouteHandler(handler: unknown): boolean { return typeof handler === 'function' && !isMiddleware(handler); } -// Read the request's own matched routes rather than importing the `hono/route` helpers, so the -// instrumentation needs no runtime import from `hono`. `c.req.matchedRoutes` and `c.req.routePath` -// are the (deprecated but public) getters those helpers wrap; `routePath(c, index)` is reimplemented -// here as `matchedRoutes(c).at(index)?.path` so an arbitrary index (e.g. -1) can be resolved. -function matchedRoutes(context: Context): HonoRoute[] { - return context.req.matchedRoutes ?? []; -} - -function routePath(context: Context, index?: number): string { - const routes = matchedRoutes(context); - return routes.at(index ?? context.req.routeIndex)?.path ?? ''; +// `c.req.matchedRoutes` exists on every Hono 4; Hono 5 removes it. +function readMatchedRoutes(context: Context): HonoRoute[] { + return context.req.matchedRoutes ?? getMatchedHandlers(context)?.map(([[, route]]) => route) ?? []; } /** - * Resolves the route path of the matched handler for the transaction name. + * Resolves the route path of the handler that responded, for the transaction name. + * + * Picking the handler (not just the last matched route) avoids two failure modes: a catch-all + * registered after the handlers (the last route would be just `/*`), and a middleware that + * short-circuits before the handler runs (`routeIndex` would point to the middleware). * - * Picking the handler (not just `routePath`) avoids two failure modes: a catch-all middleware - * registered after the handlers (`routePath(c, -1)` would return just `/*`), and a middleware that - * short-circuits before the handler runs (`routePath(c)` would return the middleware's path). + * Returns `undefined` when the request has no matched routes. */ -export function resolveRouteName(context: Context): string { - const routes = matchedRoutes(context); +export function resolveRouteName(context: Context): string | undefined { + const routes = readMatchedRoutes(context); + + const recordedIndex = getRecordedRouteHandlerIndex(context); + const recorded = recordedIndex === undefined ? undefined : routes[recordedIndex]; + if (recorded) { + return recorded.path; + } - // Trust routeIndex when it lands on a handler - to disambiguate overlapping handlers. + // No recorded index on Hono 4.0 to 4.7 with the manual setup. There, `routeIndex` stays on the + // handler that responded. const current = routes[context.req.routeIndex]; if (current && isRouteHandler(current.handler)) { return current.path; } - // A middleware short-circuited, so routeIndex is stuck on it: fall back to the last matched handler. + // No route handler started (a middleware short-circuited, or a 404): fall back to the last matched handler. for (let i = routes.length - 1; i >= 0; i--) { const route = routes[i]; if (route && isRouteHandler(route.handler)) { @@ -46,5 +48,5 @@ export function resolveRouteName(context: Context): string { // No handler matched (middleware-only path) // Final matched route: https://hono.dev/docs/helpers/route#using-with-index-parameter - return routePath(context, -1); + return routes.at(-1)?.path; } From 4a75cd3b63a9869348e0d444b7c6debb5ffad961 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 16 Oct 2026 14:44:01 +0200 Subject: [PATCH 7/8] ref(hono): Shorten comments Co-Authored-By: Claude Opus 5.5 --- .../suites/hono-sdk/scenario.mjs | 14 +++++------ .../suites/hono/scenario-honoMiddleware.mjs | 7 +++--- .../suites/hono/scenario.mjs | 12 ++++----- .../suites/hono/test.ts | 3 +-- .../hono/test/shared/resolveRouteName.test.ts | 18 ++++++------- .../src/integrations/hono/matchResult.ts | 25 ++++++++----------- .../integrations/hono/middlewareHandlers.ts | 6 ++--- .../src/integrations/hono/resolveRouteName.ts | 15 +++++------ 8 files changed, 43 insertions(+), 57 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs b/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs index 595d9546c52c..d083785435e8 100644 --- a/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs @@ -20,17 +20,16 @@ app.get('/error/:param', () => { throw new Error('Test error from Hono app'); }); -// Route-name cases. `/users/me` and `/users/:id` overlap; the request is named after the handler that -// responded, not after the last one that matched. +// Both routes match `/users/me`. The name must come from the handler that responded. app.get('/users/me', c => c.text('me')); app.get('/users/:id', c => c.text(`user ${c.req.param('id')}`)); -// A middleware that returns early: no route handler runs, so the name falls back to the last matched +// A middleware that returns early. No route handler runs, so the name falls back to the last matched // route handler. app.use('/early/*', async (c, _next) => c.text('blocked by middleware')); app.get('/early/:id', c => c.text('never reached')); -// A middleware-only path: no route handler matches at all. +// Only a middleware matches this path. app.use('/mw-only/*', async (_c, _next) => new Response('handled by middleware')); const subApp = new Hono(); @@ -41,8 +40,8 @@ app.basePath('/base').get('/hello', c => c.text('base hello')); app.all('/any-method', c => c.text(`any method: ${c.req.method}`)); -// Hono answers `HEAD` with the `GET` route. The SDK records no server span for `HEAD`, so the route -// name is checked on the error event. +// Hono answers `HEAD` with the `GET` route. The SDK records no server span for `HEAD`, so the test +// checks the name on the error event. app.get('/head-error/:id', () => { throw new Error('Test error from a HEAD request'); }); @@ -54,8 +53,7 @@ app.get('/outer/:itemId', async c => { return c.json({ outer: c.req.param('itemId'), inner: await res.json() }); }); -// A catch-all registered last. It also matches `/users/me` and `/users/:id`, but must not take over -// their names. +// A catch-all registered last. It also matches `/users/me` and `/users/42`, but must not name them. app.get('/users/*', c => c.text('users catch-all')); serve({ fetch: app.fetch, port: 0 }, info => { diff --git a/dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs b/dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs index 4e2f624b3319..ae4197efd542 100644 --- a/dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs +++ b/dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs @@ -7,10 +7,9 @@ import { Hono } from 'hono'; const app = new Hono(); app.use(Sentry.honoMiddleware(app)); -// Counts the Sentry wrappers around the running handler. Two SDK paths wrap route handlers here, and -// the router hands both the same cached handler list on each request, so a missing guard would nest one -// more wrapper per request. The match result is read through the only symbol-keyed getter on the -// request prototype (the one that `hono/route` uses). +// Counts the Sentry wrappers around the handler that runs. Both setups wrap route handlers, and the +// router reuses the handler list for each request, so without a guard the wrappers would pile up. +// It reads the match result like `hono/route` does. function wrapperCount(c) { const [matchResultKey] = Object.getOwnPropertySymbols(Object.getPrototypeOf(c.req)); let handler = c.req[matchResultKey][0][c.req.routeIndex][0][0]; diff --git a/dev-packages/node-integration-tests/suites/hono/scenario.mjs b/dev-packages/node-integration-tests/suites/hono/scenario.mjs index 4beb7f97f72d..4540dc78f1d3 100644 --- a/dev-packages/node-integration-tests/suites/hono/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/hono/scenario.mjs @@ -97,26 +97,24 @@ app }) .get('/hello', c => c.json({ greeting: 'world' })); -// Route-name cases. `/users/me` and `/users/:id` overlap; the request is named after the handler that -// responded, not after the last one that matched. +// Both routes match `/users/me`. The name must come from the handler that responded. app.get('/users/me', c => c.text('me')); app.get('/users/:id', c => c.text(`user ${c.req.param('id')}`)); -// A middleware that returns early: no route handler runs, so the name falls back to the last matched +// A middleware that returns early. No route handler runs, so the name falls back to the last matched // route handler. app.use('/early/*', async (c, _next) => c.text('blocked by middleware')); app.get('/early/:id', c => c.text('never reached')); app.all('/any-method', c => c.text(`any method: ${c.req.method}`)); -// Hono answers `HEAD` with the `GET` route. The SDK records no server span for `HEAD`, so the route -// name is checked on the error event. +// Hono answers `HEAD` with the `GET` route. The SDK records no server span for `HEAD`, so the test +// checks the name on the error event. app.get('/head-error/:id', () => { throw new Error('Test error from a HEAD request'); }); -// A catch-all registered last. It also matches `/users/me` and `/users/:id`, but must not take over -// their names. +// A catch-all registered last. It also matches `/users/me` and `/users/42`, but must not name them. app.get('/users/*', c => c.text('users catch-all')); serve({ fetch: app.fetch, port: 0 }, info => { diff --git a/dev-packages/node-integration-tests/suites/hono/test.ts b/dev-packages/node-integration-tests/suites/hono/test.ts index 7fb2dd540d85..7cf6295dc577 100644 --- a/dev-packages/node-integration-tests/suites/hono/test.ts +++ b/dev-packages/node-integration-tests/suites/hono/test.ts @@ -33,7 +33,6 @@ const ROUTE_NAME_CASES: Array<{ method: 'get' | 'post'; path: string; name: stri { method: 'get', path: '/outer/self-watering-plant', name: 'GET /outer/:itemId', route: '/outer/:itemId' }, ]; -// Matches the span container whose segment has this name, and checks its route. function segmentNamed(name: string, route: string): { span: (container: { items: StreamedSpan[] }) => void } { return { span: container => { @@ -361,7 +360,7 @@ describe('hono auto-instrumentation with honoMiddleware(app)', () => { .expect(segmentNamed('GET /users/:id', '/users/:id')) .start(); - // The router returns the same cached handler list for each request to a route. + // The second request gets the same cached handler list. expect(await runner.makeRequest('get', '/users/me')).toEqual({ wrappers: 1 }); expect(await runner.makeRequest('get', '/users/me')).toEqual({ wrappers: 1 }); expect(await runner.makeRequest('get', '/users/42')).toEqual({ wrappers: 1 }); diff --git a/packages/hono/test/shared/resolveRouteName.test.ts b/packages/hono/test/shared/resolveRouteName.test.ts index 367719eff027..e53f1f57d537 100644 --- a/packages/hono/test/shared/resolveRouteName.test.ts +++ b/packages/hono/test/shared/resolveRouteName.test.ts @@ -20,8 +20,8 @@ vi.mock('@sentry/core', async () => { const getIsolationScopeMock = SentryCore.getIsolationScope as ReturnType; -// Each request gets its own isolation scope, so the middleware's per-request dedup marker never leaks -// into the next request. +// One isolation scope per request, as in a server. A shared scope keeps the dedup marker of the +// Sentry middleware, which then skips the next request. function freshIsolationScope(): void { getIsolationScopeMock.mockReturnValue({ setTransactionName: mockSetTransactionName, @@ -35,8 +35,7 @@ function createApp({ restoreRouteIndex }: { restoreRouteIndex: boolean }): Hono app.use(createHonoRequestMiddleware()); if (restoreRouteIndex) { - // Hono 5's `compose` restores `routeIndex` after `next()` (honojs/hono#4424), so the Sentry - // middleware no longer sees the index of the handler that responded. + // Like Hono 5, which resets `routeIndex` after `next()` (honojs/hono#4424). app.use(async (c, next) => { const routeIndex = c.req.routeIndex; await next(); @@ -47,7 +46,7 @@ function createApp({ restoreRouteIndex }: { restoreRouteIndex: boolean }): Hono app.use('/users/*', async (_c, next) => { await next(); }); - // A route handler registered with `.use()`, which also gives it a middleware span wrapper. + // A route handler from `.use()`, which also gets a middleware span wrapper. app.use('/users/via-use', async c => c.text('via use')); app.get('/users/me', c => c.text('me')); app.get('/users/:id', c => c.text('user')); @@ -57,13 +56,12 @@ function createApp({ restoreRouteIndex }: { restoreRouteIndex: boolean }): Hono subApp.get('/items/:itemId', c => c.text('item')); app.route('/sub', subApp); - // A catch-all registered last also matches `/users/me` and `/users/42`, but never responds to them. + // A catch-all registered last. It also matches `/users/me` and `/users/42`, but never responds. app.get('/users/*', c => c.text('catch-all')); return app; } -// Changes a getter of the real Hono 4 request, so that the request looks like one of another Hono. -// Returns the function that restores it. +// Returns a function that restores the property. function overrideRequestProperty(key: PropertyKey, descriptor: PropertyDescriptor): () => void { const original = Object.getOwnPropertyDescriptor(HonoRequest.prototype, key); Object.defineProperty(HonoRequest.prototype, key, { configurable: true, ...descriptor }); @@ -76,7 +74,7 @@ function overrideRequestProperty(key: PropertyKey, descriptor: PropertyDescripto const hideRequestProperty = (key: PropertyKey): (() => void) => overrideRequestProperty(key, { value: undefined }); -// The key of the match result getter that `hono/route` uses (an unregistered `Symbol()`). +// The symbol of the match result getter, which `hono/route` reads. const [matchResultKey] = Object.getOwnPropertySymbols(HonoRequest.prototype); if (!matchResultKey) { throw new Error('The match result getter is missing on `HonoRequest.prototype`.'); @@ -84,7 +82,7 @@ if (!matchResultKey) { const SOURCES: Array<{ source: string; hidden: PropertyKey[]; restoreRouteIndex: boolean }> = [ { source: '`c.req.matchedRoutes` (Hono 4)', hidden: [], restoreRouteIndex: false }, - // Hono 5 has no `c.req.matchedRoutes`, so the SDK reads the match result getter. + // Hono 5 has no `c.req.matchedRoutes`. { source: 'the match result getter (Hono 5)', hidden: ['matchedRoutes'], restoreRouteIndex: true }, ]; diff --git a/packages/server-utils/src/integrations/hono/matchResult.ts b/packages/server-utils/src/integrations/hono/matchResult.ts index 4a6982ccf640..844233d61e34 100644 --- a/packages/server-utils/src/integrations/hono/matchResult.ts +++ b/packages/server-utils/src/integrations/hono/matchResult.ts @@ -2,14 +2,13 @@ import { addNonEnumerableProperty, isPlainObject } from '@sentry/core'; import type { Context, HonoRoute, MatchedHandler } from './honoTypes'; import { getMiddlewareFlags } from './isMiddleware'; -// The `routeIndex` of the last route handler that started for a request, stored on its `Context`. -// `Symbol.for` so that every copy of `@sentry/server-utils` reads the index that another copy recorded. +// The `Context` key for the `routeIndex` of the last route handler that started. `Symbol.for`, so that +// other copies of `@sentry/server-utils` can read it. const ROUTE_HANDLER_INDEX = Symbol.for('sentry.hono.routeHandlerIndex'); -// Marks the wrapper that records `ROUTE_HANDLER_INDEX`. The router returns the same cached match -// result for each request to a route, so this keeps every copy of `@sentry/server-utils` from wrapping -// a handler twice. A span wrapper does not count: a route handler that `app.use()` wrapped for a span -// must still record its index. +// Marks the index wrapper, so that a handler gets it only once. The router reuses the same match result +// for each request to a route. Span wrappers do not count, because a route handler from `app.use()` +// has a span wrapper and still needs the index wrapper. const RECORDS_ROUTE_HANDLER_INDEX = Symbol.for('sentry.hono.recordsRouteHandlerIndex'); function isMatchedHandler(entry: unknown): entry is MatchedHandler { @@ -37,9 +36,8 @@ export function isMatchedHandlerList(handlers: unknown): handlers is MatchedHand } /** - * Reads the request's match result through the getter that `hono/route` uses (Hono 4.8+ and Hono 5). - * Its key is an unregistered `Symbol()` that `hono` does not export, so the getter is found by its - * shape: it is the only symbol-keyed getter on the `HonoRequest` prototype. + * Reads the match result through the getter that `hono/route` uses (Hono 4.8 and later). `hono` does + * not export the getter's symbol, so this finds it as the only symbol-keyed getter on the prototype. */ export function getMatchedHandlers(context: Context): MatchedHandler[] | undefined { const requestProto: object | null = Object.getPrototypeOf(context.req); @@ -59,12 +57,11 @@ export function getMatchedHandlers(context: Context): MatchedHandler[] | undefin } /** - * Wraps the route handlers of a match result, so that each records `c.req.routeIndex` when it starts. - * Inside a route handler the index is correct on every Hono major, but Hono 5 restores it after - * `next()`, so the Sentry middleware cannot read it there. + * Wraps each route handler, so that it stores `c.req.routeIndex` when it starts. Hono 5 resets the + * index after `next()`, so the Sentry middleware cannot read it there. * - * `compose` reads each handler from the match result when it calls it, so a wrap from inside the - * Sentry middleware still covers the handlers that run after it. + * Calling this from the Sentry middleware itself works, because `compose` reads each handler from the + * match result only when it calls it. */ export function wrapRouteHandlers(handlers: MatchedHandler[]): void { const middlewareFlags = getMiddlewareFlags(handlers.map(([[, route]]) => route)); diff --git a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts index f6f208b2b792..ecc7583ebda3 100644 --- a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts +++ b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts @@ -24,7 +24,7 @@ import { HTTP_ROUTE } from '@sentry/conventions/attributes'; export function requestHandler(context: Context, getConnInfo?: GetConnInfo): void { const isolationScope = getCurrentIsolationScope(); - // Without the Context hook (manual setup), this is the only place that sees the match result. + // The manual setup has no Context hook, so wrap the route handlers here. const matchedHandlers = getMatchedHandlers(context); if (matchedHandlers) { wrapRouteHandlers(matchedHandlers); @@ -94,7 +94,7 @@ export function responseHandler( context: Context, shouldHandleError?: SentryHonoMiddlewareOptions['shouldHandleError'], ): void { - // Re-resolve now that the middleware chain has run and the responding route handler, if any, has recorded its index + // Resolve the name again, now that a route handler may have stored its index updateSpanRouteName(getCurrentIsolationScope(), context); captureContextError(context, shouldHandleError); @@ -123,7 +123,7 @@ export function captureContextError( function updateSpanRouteName(isolationScope: Scope, context: Context): void { const route = resolveRouteName(context); - // Keep the runtime's name rather than `GET `. + // Without a route, keep the span name from the runtime. if (route === undefined) { return; } diff --git a/packages/server-utils/src/integrations/hono/resolveRouteName.ts b/packages/server-utils/src/integrations/hono/resolveRouteName.ts index 396b6e985131..bafe18df8d0d 100644 --- a/packages/server-utils/src/integrations/hono/resolveRouteName.ts +++ b/packages/server-utils/src/integrations/hono/resolveRouteName.ts @@ -8,19 +8,16 @@ function isRouteHandler(handler: unknown): boolean { return typeof handler === 'function' && !isMiddleware(handler); } -// `c.req.matchedRoutes` exists on every Hono 4; Hono 5 removes it. +// Hono 5 has no `c.req.matchedRoutes`. function readMatchedRoutes(context: Context): HonoRoute[] { return context.req.matchedRoutes ?? getMatchedHandlers(context)?.map(([[, route]]) => route) ?? []; } /** - * Resolves the route path of the handler that responded, for the transaction name. + * Returns the route path of the handler that responded, or `undefined` if no route matched. * - * Picking the handler (not just the last matched route) avoids two failure modes: a catch-all - * registered after the handlers (the last route would be just `/*`), and a middleware that - * short-circuits before the handler runs (`routeIndex` would point to the middleware). - * - * Returns `undefined` when the request has no matched routes. + * The last matched route is not enough, because a catch-all registered later would name every + * request `/*`. */ export function resolveRouteName(context: Context): string | undefined { const routes = readMatchedRoutes(context); @@ -31,14 +28,14 @@ export function resolveRouteName(context: Context): string | undefined { return recorded.path; } - // No recorded index on Hono 4.0 to 4.7 with the manual setup. There, `routeIndex` stays on the + // Without a recorded index (manual setup before Hono 4.8), use `routeIndex`. Hono 4 leaves it on the // handler that responded. const current = routes[context.req.routeIndex]; if (current && isRouteHandler(current.handler)) { return current.path; } - // No route handler started (a middleware short-circuited, or a 404): fall back to the last matched handler. + // No route handler ran, for example because a middleware returned early. for (let i = routes.length - 1; i >= 0; i--) { const route = routes[i]; if (route && isRouteHandler(route.handler)) { From 45439846c3b163be11f44eb6bbccfd7149101fe9 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:52:38 +0200 Subject: [PATCH 8/8] fix(hono): Name the request after the route handler that responds - Name the span when a route handler starts, so events captured in the handler get its route. - Fall back to the first matched route handler, then to a middleware path. A request without a route keeps the runtime name. - Wrap route handlers only when one of them declares `next`. --- .../hono-4-legacy/package.json | 1 + .../src/route-groups/test-route-patterns.ts | 21 ++++++ .../tests/route-patterns.test.ts | 27 ++++++- .../test-applications/hono-4/package.json | 1 + .../src/route-groups/test-route-patterns.ts | 21 ++++++ .../hono-4/tests/route-patterns.test.ts | 27 ++++++- .../suites/hono-sdk/scenario.mjs | 22 +++++- .../suites/hono-sdk/test.ts | 36 ++++++++++ .../suites/hono/scenario-honoMiddleware.mjs | 12 +++- .../suites/hono/scenario.mjs | 26 +++++++ .../suites/hono/test.ts | 72 +++++++++++++++++-- .../hono/test/shared/resolveRouteName.test.ts | 59 ++++++++++++++- .../src/integrations/hono/constants.ts | 6 ++ .../integrations/hono/createHonoMiddleware.ts | 6 +- .../src/integrations/hono/honoIntegration.ts | 8 +-- .../src/integrations/hono/matchResult.ts | 45 +++++++++--- .../integrations/hono/middlewareHandlers.ts | 42 ++++++----- .../src/integrations/hono/resolveRouteName.ts | 47 ++++++------ .../integrations/hono/wrapMiddlewareSpan.ts | 2 +- 19 files changed, 404 insertions(+), 77 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/package.json b/dev-packages/e2e-tests/test-applications/hono-4-legacy/package.json index 231900020d4f..4cca12406302 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4-legacy/package.json +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/package.json @@ -16,6 +16,7 @@ "dependencies": { "@sentry/bun": "latest || *", "@sentry/cloudflare": "latest || *", + "@sentry/core": "latest || *", "@sentry/deno": "latest || *", "@sentry/hono": "latest || *", "@sentry/node": "latest || *", diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/src/route-groups/test-route-patterns.ts b/dev-packages/e2e-tests/test-applications/hono-4-legacy/src/route-groups/test-route-patterns.ts index ac6fea1320cf..0cc0aa43ed23 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4-legacy/src/route-groups/test-route-patterns.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/src/route-groups/test-route-patterns.ts @@ -1,3 +1,4 @@ +import { captureMessage } from '@sentry/core'; import { Hono } from 'hono'; const routePatterns = new Hono(); @@ -30,4 +31,24 @@ METHODS.forEach(method => { routePatterns.on(method, '/on', c => c.text(`${method} on response`)); }); +// The catch-all below also matches. An event from inside this handler must still get this route. +routePatterns.get('/capture/:id', c => { + captureMessage('Message from a route handler'); + return c.text('captured'); +}); +routePatterns.get('/capture/*', c => c.text('capture catch-all')); + +// Passes `draft` on to the catch-all below with `next()`. The catch-all responds, so it names the request. +routePatterns.get('/posts/:id', async (c, next) => { + if (c.req.param('id') === 'draft') { + await next(); + return; + } + return c.text('post'); +}); +routePatterns.get('/posts/*', c => { + captureMessage('Message from a route handler after next()'); + return c.text('fallback'); +}); + export { routePatterns }; diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/route-patterns.test.ts b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/route-patterns.test.ts index bfebb9f5b6ab..d1251fad340e 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/route-patterns.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/route-patterns.test.ts @@ -1,5 +1,10 @@ import { expect, test } from '@playwright/test'; -import { waitForStreamedSpan, getSpanOp, collectStreamedSpansUntilSegment } from '@sentry-internal/test-utils'; +import { + waitForError, + waitForStreamedSpan, + getSpanOp, + collectStreamedSpansUntilSegment, +} from '@sentry-internal/test-utils'; import { APP_NAME } from './constants'; const PREFIX = '/test-routes'; @@ -176,3 +181,23 @@ test('async handler sends span', async ({ baseURL }) => { const middlewareSpans = spans.filter(s => getSpanOp(s) === 'middleware'); expect(middlewareSpans).toEqual([]); }); + +test('names an event captured in a route handler after that route', async ({ baseURL }) => { + const eventPromise = waitForError(APP_NAME, event => event.message === 'Message from a route handler'); + + const response = await fetch(`${baseURL}${PREFIX}/capture/42`); + expect(response.status).toBe(200); + + const event = await eventPromise; + expect(event.transaction).toBe(`GET ${PREFIX}/capture/:id`); +}); + +test('names an event from the route handler that runs after next() after that route', async ({ baseURL }) => { + const eventPromise = waitForError(APP_NAME, event => event.message === 'Message from a route handler after next()'); + + const response = await fetch(`${baseURL}${PREFIX}/posts/draft`); + expect(response.status).toBe(200); + + const event = await eventPromise; + expect(event.transaction).toBe(`GET ${PREFIX}/posts/*`); +}); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/package.json b/dev-packages/e2e-tests/test-applications/hono-4/package.json index 6aba3043d05f..a608d1775ef6 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/package.json +++ b/dev-packages/e2e-tests/test-applications/hono-4/package.json @@ -20,6 +20,7 @@ "dependencies": { "@sentry/bun": "latest || *", "@sentry/cloudflare": "latest || *", + "@sentry/core": "latest || *", "@sentry/deno": "latest || *", "@sentry/node": "latest || *", "@hono/node-server": "^2.0.5", diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-route-patterns.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-route-patterns.ts index ac6fea1320cf..0cc0aa43ed23 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-route-patterns.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-route-patterns.ts @@ -1,3 +1,4 @@ +import { captureMessage } from '@sentry/core'; import { Hono } from 'hono'; const routePatterns = new Hono(); @@ -30,4 +31,24 @@ METHODS.forEach(method => { routePatterns.on(method, '/on', c => c.text(`${method} on response`)); }); +// The catch-all below also matches. An event from inside this handler must still get this route. +routePatterns.get('/capture/:id', c => { + captureMessage('Message from a route handler'); + return c.text('captured'); +}); +routePatterns.get('/capture/*', c => c.text('capture catch-all')); + +// Passes `draft` on to the catch-all below with `next()`. The catch-all responds, so it names the request. +routePatterns.get('/posts/:id', async (c, next) => { + if (c.req.param('id') === 'draft') { + await next(); + return; + } + return c.text('post'); +}); +routePatterns.get('/posts/*', c => { + captureMessage('Message from a route handler after next()'); + return c.text('fallback'); +}); + export { routePatterns }; diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts index bfebb9f5b6ab..d1251fad340e 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts @@ -1,5 +1,10 @@ import { expect, test } from '@playwright/test'; -import { waitForStreamedSpan, getSpanOp, collectStreamedSpansUntilSegment } from '@sentry-internal/test-utils'; +import { + waitForError, + waitForStreamedSpan, + getSpanOp, + collectStreamedSpansUntilSegment, +} from '@sentry-internal/test-utils'; import { APP_NAME } from './constants'; const PREFIX = '/test-routes'; @@ -176,3 +181,23 @@ test('async handler sends span', async ({ baseURL }) => { const middlewareSpans = spans.filter(s => getSpanOp(s) === 'middleware'); expect(middlewareSpans).toEqual([]); }); + +test('names an event captured in a route handler after that route', async ({ baseURL }) => { + const eventPromise = waitForError(APP_NAME, event => event.message === 'Message from a route handler'); + + const response = await fetch(`${baseURL}${PREFIX}/capture/42`); + expect(response.status).toBe(200); + + const event = await eventPromise; + expect(event.transaction).toBe(`GET ${PREFIX}/capture/:id`); +}); + +test('names an event from the route handler that runs after next() after that route', async ({ baseURL }) => { + const eventPromise = waitForError(APP_NAME, event => event.message === 'Message from a route handler after next()'); + + const response = await fetch(`${baseURL}${PREFIX}/posts/draft`); + expect(response.status).toBe(200); + + const event = await eventPromise; + expect(event.transaction).toBe(`GET ${PREFIX}/posts/*`); +}); diff --git a/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs b/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs index d083785435e8..0456605b5a13 100644 --- a/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/hono-sdk/scenario.mjs @@ -1,5 +1,5 @@ import { serve } from '@hono/node-server'; -import { sentry } from '@sentry/hono/node'; +import { captureMessage, sentry } from '@sentry/hono/node'; import { sendPortToRunner } from '@sentry-internal/node-integration-tests'; import { Hono } from 'hono'; @@ -24,6 +24,12 @@ app.get('/error/:param', () => { app.get('/users/me', c => c.text('me')); app.get('/users/:id', c => c.text(`user ${c.req.param('id')}`)); +// The catch-all below also matches. An event from inside this handler must still get this route. +app.get('/users/:id/capture', c => { + captureMessage('Message from a route handler'); + return c.text('captured'); +}); + // A middleware that returns early. No route handler runs, so the name falls back to the last matched // route handler. app.use('/early/*', async (c, _next) => c.text('blocked by middleware')); @@ -53,8 +59,22 @@ app.get('/outer/:itemId', async c => { return c.json({ outer: c.req.param('itemId'), inner: await res.json() }); }); +// Passes `draft` on to the catch-all below with `next()`. The catch-all responds, so it names the request. +app.get('/posts/:id', async (c, next) => { + if (c.req.param('id') === 'draft') { + await next(); + return; + } + return c.text('post'); +}); +app.get('/posts/*', c => { + captureMessage('Message from a route handler after next()'); + return c.text('fallback'); +}); + // A catch-all registered last. It also matches `/users/me` and `/users/42`, but must not name them. app.get('/users/*', c => c.text('users catch-all')); +app.get('/early/*', c => c.text('never reached')); serve({ fetch: app.fetch, port: 0 }, info => { sendPortToRunner(info.port); diff --git a/dev-packages/node-integration-tests/suites/hono-sdk/test.ts b/dev-packages/node-integration-tests/suites/hono-sdk/test.ts index 160ec4c81892..ca34ff88baf8 100644 --- a/dev-packages/node-integration-tests/suites/hono-sdk/test.ts +++ b/dev-packages/node-integration-tests/suites/hono-sdk/test.ts @@ -6,6 +6,8 @@ const ROUTE_NAME_CASES: Array<{ method: 'get' | 'post'; path: string; name: stri { method: 'get', path: '/users/42', name: 'GET /users/:id', route: '/users/:id' }, { method: 'get', path: '/users/42/posts', name: 'GET /users/*', route: '/users/*' }, { method: 'get', path: '/early/1', name: 'GET /early/:id', route: '/early/:id' }, + { method: 'get', path: '/posts/1', name: 'GET /posts/:id', route: '/posts/:id' }, + { method: 'get', path: '/posts/draft', name: 'GET /posts/*', route: '/posts/*' }, { method: 'get', path: '/mw-only/anything', name: 'GET /mw-only/*', route: '/mw-only/*' }, { method: 'get', path: '/sub/hello', name: 'GET /sub/hello', route: '/sub/hello' }, { method: 'get', path: '/base/hello', name: 'GET /base/hello', route: '/base/hello' }, @@ -42,6 +44,40 @@ describe('hono-sdk (Node)', () => { await started.completed(); }); + test('names an event captured in a route handler after that route', async () => { + const runner = createRunner() + .ignore('span') + .expect({ event: { message: 'Message from a route handler', transaction: 'GET /users/:id/capture' } }) + .start(); + runner.makeRequest('get', '/users/42/capture'); + await runner.completed(); + }); + + test('keeps the runtime name for a request without a route', async () => { + const runner = createRunner() + .expect({ + span: container => { + const segment = container.items.find(item => item.is_segment); + expect(segment?.name).toBe('GET'); + expect(segment?.attributes?.['sentry.segment.name.source']).toEqual({ type: 'string', value: 'url' }); + expect(segment?.attributes?.['url.path']).toEqual({ type: 'string', value: '/does-not-exist' }); + expect(segment?.attributes?.['http.route']).toBeUndefined(); + }, + }) + .start(); + runner.makeRequest('get', '/does-not-exist', { expectError: true }); + await runner.completed(); + }); + + test('names an event from the route handler that runs after next() after that route', async () => { + const runner = createRunner() + .ignore('span') + .expect({ event: { message: 'Message from a route handler after next()', transaction: 'GET /posts/*' } }) + .start(); + runner.makeRequest('get', '/posts/draft'); + await runner.completed(); + }); + test('names a HEAD request after the GET route', async () => { const runner = createRunner() .ignore('span') diff --git a/dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs b/dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs index ae4197efd542..d6d6c32a9a1d 100644 --- a/dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs +++ b/dev-packages/node-integration-tests/suites/hono/scenario-honoMiddleware.mjs @@ -21,8 +21,18 @@ function wrapperCount(c) { return count; } +// No route handler here declares `next`, so the SDK does not wrap them. app.get('/users/me', c => c.json({ wrappers: wrapperCount(c) })); -app.get('/users/:id', c => c.json({ wrappers: wrapperCount(c) })); + +// A route handler that declares `next`, so the SDK wraps the route handlers of this request. +app.get('/posts/:id', async (c, next) => { + if (c.req.param('id') === 'draft') { + await next(); + return; + } + return c.json({ wrappers: wrapperCount(c) }); +}); +app.get('/posts/*', c => c.json({ wrappers: wrapperCount(c) })); serve({ fetch: app.fetch, port: 0 }, info => { sendPortToRunner(info.port); diff --git a/dev-packages/node-integration-tests/suites/hono/scenario.mjs b/dev-packages/node-integration-tests/suites/hono/scenario.mjs index 4540dc78f1d3..e1c609d1a454 100644 --- a/dev-packages/node-integration-tests/suites/hono/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/hono/scenario.mjs @@ -1,5 +1,6 @@ import { serve } from '@hono/node-server'; import { sendPortToRunner } from '@sentry-internal/node-integration-tests'; +import { captureMessage, startSpan } from '@sentry/node'; import { Hono } from 'hono'; // No `@sentry/hono` and no `sentry()` middleware: the app is instrumented automatically by the @@ -101,6 +102,12 @@ app app.get('/users/me', c => c.text('me')); app.get('/users/:id', c => c.text(`user ${c.req.param('id')}`)); +// The catch-all below also matches. An event from inside this handler must still get this route. +app.get('/users/:id/capture', c => { + captureMessage('Message from a route handler'); + return c.text('captured'); +}); + // A middleware that returns early. No route handler runs, so the name falls back to the last matched // route handler. app.use('/early/*', async (c, _next) => c.text('blocked by middleware')); @@ -114,8 +121,27 @@ app.get('/head-error/:id', () => { throw new Error('Test error from a HEAD request'); }); +// Runs the rest of the chain in its own span. The SDK must name the segment, not this span. The route +// handler declares `next`, so the SDK names the request when the handler starts, inside this span. +app.use('/traced/*', (_c, next) => startSpan({ name: 'user span' }, () => next())); +app.get('/traced/:id', (c, _next) => c.text('traced')); + +// Passes `draft` on to the catch-all below with `next()`. The catch-all responds, so it names the request. +app.get('/posts/:id', async (c, next) => { + if (c.req.param('id') === 'draft') { + await next(); + return; + } + return c.text('post'); +}); +app.get('/posts/*', c => { + captureMessage('Message from a route handler after next()'); + return c.text('fallback'); +}); + // A catch-all registered last. It also matches `/users/me` and `/users/42`, but must not name them. app.get('/users/*', c => c.text('users catch-all')); +app.get('/early/*', c => c.text('never reached')); serve({ fetch: app.fetch, port: 0 }, info => { sendPortToRunner(info.port); diff --git a/dev-packages/node-integration-tests/suites/hono/test.ts b/dev-packages/node-integration-tests/suites/hono/test.ts index 7cf6295dc577..511ea3a83cf7 100644 --- a/dev-packages/node-integration-tests/suites/hono/test.ts +++ b/dev-packages/node-integration-tests/suites/hono/test.ts @@ -21,6 +21,8 @@ const ROUTE_NAME_CASES: Array<{ method: 'get' | 'post'; path: string; name: stri { method: 'get', path: '/users/42/posts', name: 'GET /users/*', route: '/users/*' }, { method: 'get', path: '/overlap/123', name: 'GET /overlap/:id', route: '/overlap/:id' }, { method: 'get', path: '/early/1', name: 'GET /early/:id', route: '/early/:id' }, + { method: 'get', path: '/posts/1', name: 'GET /posts/:id', route: '/posts/:id' }, + { method: 'get', path: '/posts/draft', name: 'GET /posts/*', route: '/posts/*' }, { method: 'get', path: '/mw-only/anything', name: 'GET /mw-only/*', route: '/mw-only/*' }, { method: 'get', path: '/sub/hello', name: 'GET /sub/hello', route: '/sub/hello' }, { @@ -67,6 +69,62 @@ describe('hono auto-instrumentation', () => { await started.completed(); }); + test('names an event captured in a route handler after that route', async () => { + const runner = createRunner() + .ignore('span') + .expect({ event: { message: 'Message from a route handler', transaction: 'GET /users/:id/capture' } }) + .start(); + runner.makeRequest('get', '/users/42/capture'); + await runner.completed(); + }); + + test('does not rename a span that a middleware starts around the route handler', async () => { + const runner = createRunner() + .unordered() + .expect({ + span: container => { + const segment = container.items.find(item => item.is_segment && item.name === 'GET /traced/:id'); + if (!segment) { + throw new Error('segment for `GET /traced/:id` not in this container'); + } + expect(container.items.filter(item => item.name === 'user span')).toHaveLength(1); + }, + }) + .start(); + runner.makeRequest('get', '/traced/1'); + await runner.completed(); + }); + + test('keeps the runtime name for a request without a route', async () => { + const runner = createRunner() + .unordered() + .expect({ + span: container => { + const segment = container.items.find( + item => item.is_segment && attr(item, 'url.path') === '/does-not-exist', + ); + if (!segment) { + throw new Error('segment for `/does-not-exist` not in this container'); + } + expect(segment.name).toBe('GET'); + expect(attr(segment, 'sentry.segment.name.source')).toBe('url'); + expect(attr(segment, 'http.route')).toBeUndefined(); + }, + }) + .start(); + runner.makeRequest('get', '/does-not-exist', { expectError: true }); + await runner.completed(); + }); + + test('names an event from the route handler that runs after next() after that route', async () => { + const runner = createRunner() + .ignore('span') + .expect({ event: { message: 'Message from a route handler after next()', transaction: 'GET /posts/*' } }) + .start(); + runner.makeRequest('get', '/posts/draft'); + await runner.completed(); + }); + test('names a HEAD request after the GET route', async () => { const runner = createRunner() .unordered() @@ -352,18 +410,20 @@ describe('hono auto-instrumentation with honoMiddleware(app)', () => { }); createEsmAndCjsTests(__dirname, 'scenario-honoMiddleware.mjs', 'instrument.mjs', (createRunner, test) => { - test('wraps each route handler once and names the request after it', async () => { + test('wraps route handlers only when one declares next, and only once', async () => { const runner = createRunner() .unordered() .expect(segmentNamed('GET /users/me', '/users/me')) - .expect(segmentNamed('GET /users/me', '/users/me')) - .expect(segmentNamed('GET /users/:id', '/users/:id')) + .expect(segmentNamed('GET /posts/:id', '/posts/:id')) + .expect(segmentNamed('GET /posts/:id', '/posts/:id')) + .expect(segmentNamed('GET /posts/*', '/posts/*')) .start(); + expect(await runner.makeRequest('get', '/users/me')).toEqual({ wrappers: 0 }); // The second request gets the same cached handler list. - expect(await runner.makeRequest('get', '/users/me')).toEqual({ wrappers: 1 }); - expect(await runner.makeRequest('get', '/users/me')).toEqual({ wrappers: 1 }); - expect(await runner.makeRequest('get', '/users/42')).toEqual({ wrappers: 1 }); + expect(await runner.makeRequest('get', '/posts/1')).toEqual({ wrappers: 1 }); + expect(await runner.makeRequest('get', '/posts/1')).toEqual({ wrappers: 1 }); + expect(await runner.makeRequest('get', '/posts/draft')).toEqual({ wrappers: 1 }); await runner.completed(); }); }); diff --git a/packages/hono/test/shared/resolveRouteName.test.ts b/packages/hono/test/shared/resolveRouteName.test.ts index e53f1f57d537..7ecd4e337502 100644 --- a/packages/hono/test/shared/resolveRouteName.test.ts +++ b/packages/hono/test/shared/resolveRouteName.test.ts @@ -52,10 +52,26 @@ function createApp({ restoreRouteIndex }: { restoreRouteIndex: boolean }): Hono app.get('/users/:id', c => c.text('user')); app.use('/mw-only/*', async (_c, _next) => new Response('handled by middleware')); + // A middleware that returns early. Of the matched route handlers, Hono would have run `/blocked/me`. + app.use('/blocked/*', async (c, _next) => c.text('blocked', 401)); + app.get('/blocked/me', c => c.text('me')); + app.get('/blocked/:id', c => c.text('user')); + app.get('/blocked/*', c => c.text('catch-all')); + const subApp = new Hono(); subApp.get('/items/:itemId', c => c.text('item')); app.route('/sub', subApp); + // Passes `draft` on to the catch-all below with `next()`. The catch-all responds, so it names the request. + app.get('/posts/:id', async (c, next) => { + if (c.req.param('id') === 'draft') { + await next(); + return; + } + return c.text('post'); + }); + app.get('/posts/*', c => c.text('fallback')); + // A catch-all registered last. It also matches `/users/me` and `/users/42`, but never responds. app.get('/users/*', c => c.text('catch-all')); return app; @@ -81,9 +97,9 @@ if (!matchResultKey) { } const SOURCES: Array<{ source: string; hidden: PropertyKey[]; restoreRouteIndex: boolean }> = [ - { source: '`c.req.matchedRoutes` (Hono 4)', hidden: [], restoreRouteIndex: false }, + { source: 'Hono 4', hidden: [], restoreRouteIndex: false }, // Hono 5 has no `c.req.matchedRoutes`. - { source: 'the match result getter (Hono 5)', hidden: ['matchedRoutes'], restoreRouteIndex: true }, + { source: 'Hono 5 (simulated)', hidden: ['matchedRoutes'], restoreRouteIndex: true }, ]; const CASES: Array<[path: string, expected: string]> = [ @@ -93,9 +109,12 @@ const CASES: Array<[path: string, expected: string]> = [ ['/users/all/the/things', 'GET /users/*'], ['/sub/items/7', 'GET /sub/items/:itemId'], ['/mw-only/anything', 'GET /mw-only/*'], + ['/blocked/me', 'GET /blocked/me'], + ['/posts/1', 'GET /posts/:id'], + ['/posts/draft', 'GET /posts/*'], ]; -describe.each(SOURCES)('route name from $source', ({ hidden, restoreRouteIndex }) => { +describe.each(SOURCES)('route name on $source', ({ hidden, restoreRouteIndex }) => { let restores: Array<() => void> = []; beforeEach(() => { @@ -115,6 +134,40 @@ describe.each(SOURCES)('route name from $source', ({ hidden, restoreRouteIndex } expect(mockSetTransactionName).toHaveBeenLastCalledWith(expected); }); + + it('does not name a request without a route', async () => { + freshIsolationScope(); + const app = createApp({ restoreRouteIndex }); + + await app.request('/does-not-exist'); + + expect(mockSetTransactionName).not.toHaveBeenCalled(); + }); +}); + +describe('route name with another symbol-keyed getter on the request', () => { + let restores: Array<() => void> = []; + + beforeEach(() => { + vi.clearAllMocks(); + restores = [ + hideRequestProperty('matchedRoutes'), + overrideRequestProperty(Symbol('requestId'), { get: () => 'req-7f3a' }), + ]; + }); + + afterEach(() => { + restores.forEach(restore => restore()); + }); + + it('still finds the match result getter', async () => { + freshIsolationScope(); + const app = createApp({ restoreRouteIndex: true }); + + await app.request('/users/me'); + + expect(mockSetTransactionName).toHaveBeenLastCalledWith('GET /users/me'); + }); }); describe('route name from a match result of an unknown shape', () => { diff --git a/packages/server-utils/src/integrations/hono/constants.ts b/packages/server-utils/src/integrations/hono/constants.ts index 297f3a469271..243b4eaebb57 100644 --- a/packages/server-utils/src/integrations/hono/constants.ts +++ b/packages/server-utils/src/integrations/hono/constants.ts @@ -2,3 +2,9 @@ // Shared by the `app.request` Proxy (`patchAppRequest`) and the orchestrion channel subscriber // (`instrumentInternalRequests` in `honoIntegration`), which both open this span. export const INTERNAL_REQUEST_ORIGIN = 'auto.http.hono.internal_request'; + +// Marks the Sentry request/response middleware so the span-wrapping patches never turn it into a +// middleware span — most importantly when an auto-instrumented sub-app carrying this middleware is +// mounted into a parent and `wrapSubAppMiddleware` wraps its handlers. Read by `wrapMiddlewareWithSpan` +// and by `resolveRouteName`, which does not use its path as a route. +export const SENTRY_HONO_MIDDLEWARE = '__SENTRY_HONO_MIDDLEWARE__'; diff --git a/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts b/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts index 74f0a96ca809..3f1a2a78892c 100644 --- a/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts +++ b/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts @@ -1,13 +1,9 @@ import { addNonEnumerableProperty, getDefaultIsolationScope, getIsolationScope } from '@sentry/core'; +import { SENTRY_HONO_MIDDLEWARE } from './constants'; import type { Context, GetConnInfo, MiddlewareHandler } from './honoTypes'; import { captureContextError, requestHandler, responseHandler } from './middlewareHandlers'; import type { SentryHonoMiddlewareOptions } from './types'; -// Marks the Sentry request/response middleware so the span-wrapping patches never turn it into a -// middleware span — most importantly when an auto-instrumented sub-app carrying this middleware is -// mounted into a parent and `wrapSubAppMiddleware` wraps its handlers. Read by `wrapMiddlewareWithSpan`. -export const SENTRY_HONO_MIDDLEWARE = '__SENTRY_HONO_MIDDLEWARE__'; - // `Symbol.for` (global registry) so the markers are shared even if this module is evaluated from more // than one copy of `@sentry/server-utils`. const HONO_REQUEST_HANDLED = Symbol.for('sentry.hono.requestHandled'); diff --git a/packages/server-utils/src/integrations/hono/honoIntegration.ts b/packages/server-utils/src/integrations/hono/honoIntegration.ts index 653ac895f456..7fd32383796a 100644 --- a/packages/server-utils/src/integrations/hono/honoIntegration.ts +++ b/packages/server-utils/src/integrations/hono/honoIntegration.ts @@ -84,8 +84,7 @@ export function honoMiddleware(app: Hono, options: HonoIntegra }); } -function wrapMatchedMiddleware(handlers: MatchedHandler[]): void { - const middlewareFlags = getMiddlewareFlags(handlers.map(([[, route]]) => route)); +function wrapMatchedMiddleware(handlers: MatchedHandler[], middlewareFlags: boolean[]): void { for (const [i, [pair]] of handlers.entries()) { if (middlewareFlags[i]) { pair[0] = wrapMiddlewareWithSpan(pair[0]); @@ -117,8 +116,9 @@ function injectHonoInstrumentation( injectedHandlerLists.add(handlers); if (isMatchedHandlerList(handlers)) { - wrapMatchedMiddleware(handlers); - wrapRouteHandlers(handlers); + const middlewareFlags = getMiddlewareFlags(handlers.map(([[, route]]) => route)); + wrapMatchedMiddleware(handlers, middlewareFlags); + wrapRouteHandlers(handlers, middlewareFlags); } // Prepend the Sentry request/response middleware. Its arity marks it as a middleware, so its `/*` diff --git a/packages/server-utils/src/integrations/hono/matchResult.ts b/packages/server-utils/src/integrations/hono/matchResult.ts index 844233d61e34..a277fe117c5e 100644 --- a/packages/server-utils/src/integrations/hono/matchResult.ts +++ b/packages/server-utils/src/integrations/hono/matchResult.ts @@ -1,6 +1,6 @@ import { addNonEnumerableProperty, isPlainObject } from '@sentry/core'; import type { Context, HonoRoute, MatchedHandler } from './honoTypes'; -import { getMiddlewareFlags } from './isMiddleware'; +import { isMiddleware } from './isMiddleware'; // The `Context` key for the `routeIndex` of the last route handler that started. `Symbol.for`, so that // other copies of `@sentry/server-utils` can read it. @@ -11,6 +11,9 @@ const ROUTE_HANDLER_INDEX = Symbol.for('sentry.hono.routeHandlerIndex'); // has a span wrapper and still needs the index wrapper. const RECORDS_ROUTE_HANDLER_INDEX = Symbol.for('sentry.hono.recordsRouteHandlerIndex'); +// The `Context` key for the callback that runs when a route handler starts. +const ON_ROUTE_HANDLER_START = Symbol.for('sentry.hono.onRouteHandlerStart'); + function isMatchedHandler(entry: unknown): entry is MatchedHandler { const pair: unknown = Array.isArray(entry) ? entry[0] : undefined; if (!Array.isArray(pair)) { @@ -37,7 +40,8 @@ export function isMatchedHandlerList(handlers: unknown): handlers is MatchedHand /** * Reads the match result through the getter that `hono/route` uses (Hono 4.8 and later). `hono` does - * not export the getter's symbol, so this finds it as the only symbol-keyed getter on the prototype. + * not export the getter's symbol, so this finds it as the symbol-keyed getter that returns a match + * result. */ export function getMatchedHandlers(context: Context): MatchedHandler[] | undefined { const requestProto: object | null = Object.getPrototypeOf(context.req); @@ -45,31 +49,46 @@ export function getMatchedHandlers(context: Context): MatchedHandler[] | undefin return undefined; } - const [getterKey, ...otherGetterKeys] = Object.getOwnPropertySymbols(requestProto).filter( - key => typeof Object.getOwnPropertyDescriptor(requestProto, key)?.get === 'function', - ); - if (!getterKey || otherGetterKeys.length > 0) { - return undefined; + for (const key of Object.getOwnPropertySymbols(requestProto)) { + if (typeof Object.getOwnPropertyDescriptor(requestProto, key)?.get !== 'function') { + continue; + } + const matchResult: unknown = Reflect.get(context.req, key); + if (Array.isArray(matchResult) && isMatchedHandlerList(matchResult[0])) { + return matchResult[0]; + } } + return undefined; +} - const matchResult: unknown = Reflect.get(context.req, getterKey); - return Array.isArray(matchResult) && isMatchedHandlerList(matchResult[0]) ? matchResult[0] : undefined; +/** Runs `callback` each time a route handler of this request starts. */ +export function onRouteHandlerStart(context: Context, callback: () => void): void { + addNonEnumerableProperty(context, ON_ROUTE_HANDLER_START, callback); } /** * Wraps each route handler, so that it stores `c.req.routeIndex` when it starts. Hono 5 resets the * index after `next()`, so the Sentry middleware cannot read it there. * + * Only a route handler that declares `next` can pass the request on to a later one. If none does, the + * first route handler that runs also responds, so this wraps nothing and returns `false`. + * * Calling this from the Sentry middleware itself works, because `compose` reads each handler from the * match result only when it calls it. */ -export function wrapRouteHandlers(handlers: MatchedHandler[]): void { - const middlewareFlags = getMiddlewareFlags(handlers.map(([[, route]]) => route)); +export function wrapRouteHandlers(handlers: MatchedHandler[], middlewareFlags: boolean[]): boolean { + // `isMiddleware()` checks the arity, so for a route handler it tells if the handler declares `next`. + const canPassOn = handlers.some(([[handler]], i) => !middlewareFlags[i] && isMiddleware(handler)); + if (!canPassOn) { + return false; + } + for (const [i, [pair]] of handlers.entries()) { if (!middlewareFlags[i]) { pair[0] = recordRouteHandlerIndex(pair[0]); } } + return true; } function recordRouteHandlerIndex(handler: HonoRoute['handler']): HonoRoute['handler'] { @@ -81,6 +100,10 @@ function recordRouteHandlerIndex(handler: HonoRoute['handler']): HonoRoute['hand apply(target, thisArg, args: [Context, ...unknown[]]) { const [context] = args; addNonEnumerableProperty(context, ROUTE_HANDLER_INDEX, context.req.routeIndex); + const onStart: unknown = Reflect.get(context, ON_ROUTE_HANDLER_START); + if (typeof onStart === 'function') { + onStart(); + } return Reflect.apply(target, thisArg, args); }, get(target, prop, receiver) { diff --git a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts index ecc7583ebda3..b2701495f12c 100644 --- a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts +++ b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts @@ -8,12 +8,14 @@ import { INTERNAL_setSegmentNameSourceIfSegment, updateSpanName, type Scope, + type Span, winterCGRequestToRequestData, } from '@sentry/core'; import type { Context, GetConnInfo } from './honoTypes'; import { hasFetchEvent } from './hono-context'; import { defaultShouldHandleError } from './defaultShouldHandleError'; -import { getMatchedHandlers, wrapRouteHandlers } from './matchResult'; +import { getMiddlewareFlags } from './isMiddleware'; +import { getMatchedHandlers, onRouteHandlerStart, wrapRouteHandlers } from './matchResult'; import { resolveRouteName } from './resolveRouteName'; import { type SentryHonoMiddlewareOptions } from './types'; import { HTTP_ROUTE } from '@sentry/conventions/attributes'; @@ -23,17 +25,26 @@ import { HTTP_ROUTE } from '@sentry/conventions/attributes'; */ export function requestHandler(context: Context, getConnInfo?: GetConnInfo): void { const isolationScope = getCurrentIsolationScope(); + // The span to name. When a route handler starts, a span of the user can be the active one. + const span = getActiveSpan(); - // The manual setup has no Context hook, so wrap the route handlers here. + // Hono 5 removes `c.req.matchedRoutes`. Before Hono 4.8, it is the only source. const matchedHandlers = getMatchedHandlers(context); - if (matchedHandlers) { - wrapRouteHandlers(matchedHandlers); + const routes = matchedHandlers?.map(([[, route]]) => route) ?? context.req.matchedRoutes ?? []; + const middlewareFlags = getMiddlewareFlags(routes); + const nameSpan = (): void => + updateSpanRouteName(isolationScope, span, context, resolveRouteName(context, routes, middlewareFlags)); + + // Name the span before the handlers run, so that events captured during the request carry the route. + nameSpan(); + + // The manual setup has no Context hook, so wrap the route handlers here. Without the getter, the + // Context hook may still have wrapped them. + if (!matchedHandlers || wrapRouteHandlers(matchedHandlers, middlewareFlags)) { + // A route handler can pass the request on with `next()`, so the one that responds names it. + onRouteHandlerStart(context, nameSpan); } - // Set a provisional route name as early as possible so events captured during the request already carry the route. - // It is re-resolved in `responseHandler` once the middleware chain has run. - updateSpanRouteName(isolationScope, context); - isolationScope.setSDKProcessingMetadata({ normalizedRequest: winterCGRequestToRequestData(hasFetchEvent(context) ? context.event.request : context.req.raw), }); @@ -94,17 +105,13 @@ export function responseHandler( context: Context, shouldHandleError?: SentryHonoMiddlewareOptions['shouldHandleError'], ): void { - // Resolve the name again, now that a route handler may have stored its index - updateSpanRouteName(getCurrentIsolationScope(), context); - captureContextError(context, shouldHandleError); } /** * Captures an unhandled context error, gated by the user's `shouldHandleError` (or the default). * - * Split out from {@link responseHandler} so a deduplicated middleware can still report its own - * context's error without re-resolving the route name or overwriting request data — e.g. the inner + * A deduplicated middleware calls it directly to report the error of its own context — e.g. the inner * context of an internal `.request()` whose route threw but whose failed response the outer handler * swallowed. */ @@ -121,15 +128,18 @@ export function captureContextError( } } -function updateSpanRouteName(isolationScope: Scope, context: Context): void { - const route = resolveRouteName(context); +function updateSpanRouteName( + isolationScope: Scope, + activeSpan: Span | undefined, + context: Context, + route: string | undefined, +): void { // Without a route, keep the span name from the runtime. if (route === undefined) { return; } const routeName = `${context.req.method} ${route}`; - const activeSpan = getActiveSpan(); if (activeSpan) { activeSpan.updateName(routeName); diff --git a/packages/server-utils/src/integrations/hono/resolveRouteName.ts b/packages/server-utils/src/integrations/hono/resolveRouteName.ts index bafe18df8d0d..1b9877586d0d 100644 --- a/packages/server-utils/src/integrations/hono/resolveRouteName.ts +++ b/packages/server-utils/src/integrations/hono/resolveRouteName.ts @@ -1,49 +1,42 @@ import type { Context, HonoRoute } from './honoTypes'; -import { isMiddleware } from './isMiddleware'; -import { getMatchedHandlers, getRecordedRouteHandlerIndex } from './matchResult'; - -// Arity alone is enough here (unlike `wrapSubAppMiddleware` in patchRoute.ts, which also needs position) -// We only want the path, and inline middleware shares its handler's path. -function isRouteHandler(handler: unknown): boolean { - return typeof handler === 'function' && !isMiddleware(handler); -} - -// Hono 5 has no `c.req.matchedRoutes`. -function readMatchedRoutes(context: Context): HonoRoute[] { - return context.req.matchedRoutes ?? getMatchedHandlers(context)?.map(([[, route]]) => route) ?? []; -} +import { SENTRY_HONO_MIDDLEWARE } from './constants'; +import { getRecordedRouteHandlerIndex } from './matchResult'; /** - * Returns the route path of the handler that responded, or `undefined` if no route matched. + * Returns the route path of the handler that responded, or `undefined` if the request has no route. * * The last matched route is not enough, because a catch-all registered later would name every * request `/*`. */ -export function resolveRouteName(context: Context): string | undefined { - const routes = readMatchedRoutes(context); - +export function resolveRouteName( + context: Context, + routes: HonoRoute[], + middlewareFlags: boolean[], +): string | undefined { const recordedIndex = getRecordedRouteHandlerIndex(context); const recorded = recordedIndex === undefined ? undefined : routes[recordedIndex]; if (recorded) { return recorded.path; } - // Without a recorded index (manual setup before Hono 4.8), use `routeIndex`. Hono 4 leaves it on the - // handler that responded. - const current = routes[context.req.routeIndex]; - if (current && isRouteHandler(current.handler)) { - return current.path; + // No route handler that can pass the request on ran. Hono runs the first matched route handler, or would + // have run it if a middleware had not returned early. + const firstRouteHandler = routes.find((_route, i) => !middlewareFlags[i]); + if (firstRouteHandler) { + return firstRouteHandler.path; } - // No route handler ran, for example because a middleware returned early. + // Only middleware matched, for example `app.use('/static/*', serveStatic())`. Its path is a route of + // the app. The Sentry middleware and a middleware for every path (`/*`) say nothing about the route. for (let i = routes.length - 1; i >= 0; i--) { const route = routes[i]; - if (route && isRouteHandler(route.handler)) { + if (route && route.path !== '/*' && !isSentryMiddleware(route.handler)) { return route.path; } } + return undefined; +} - // No handler matched (middleware-only path) - // Final matched route: https://hono.dev/docs/helpers/route#using-with-index-parameter - return routes.at(-1)?.path; +function isSentryMiddleware(handler: unknown): boolean { + return typeof handler === 'function' && Boolean(Reflect.get(handler, SENTRY_HONO_MIDDLEWARE)); } diff --git a/packages/server-utils/src/integrations/hono/wrapMiddlewareSpan.ts b/packages/server-utils/src/integrations/hono/wrapMiddlewareSpan.ts index 86f96e05039c..29f8d97286a4 100644 --- a/packages/server-utils/src/integrations/hono/wrapMiddlewareSpan.ts +++ b/packages/server-utils/src/integrations/hono/wrapMiddlewareSpan.ts @@ -9,7 +9,7 @@ import { type WrappedFunction, } from '@sentry/core'; import { type MiddlewareHandler } from './honoTypes'; -import { SENTRY_HONO_MIDDLEWARE } from './createHonoMiddleware'; +import { SENTRY_HONO_MIDDLEWARE } from './constants'; import { defaultShouldHandleError } from './defaultShouldHandleError'; const MIDDLEWARE_ORIGIN = 'auto.middleware.hono';