From c8b0c65ee24d1153baf7d1e36cc04e017610f610 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 12 May 2026 15:46:53 +0200 Subject: [PATCH 1/4] feat(hono): Add `hono.request` spans for internal `.request()` calls --- .../src/route-groups/test-multi-fetch.ts | 99 ++++++ .../test-applications/hono-4/src/routes.ts | 4 + .../hono-4/tests/multi-fetch.test.ts | 310 ++++++++++++++++++ packages/hono/src/index.bun.ts | 4 + packages/hono/src/index.cloudflare.ts | 4 + packages/hono/src/index.node.ts | 4 + packages/hono/src/shared/applyPatches.ts | 45 ++- packages/hono/src/shared/patchAppRequest.ts | 65 ++++ packages/hono/src/shared/patchAppUse.ts | 15 +- packages/hono/src/shared/patchRoute.ts | 87 ++++- .../hono/test/shared/applyPatches.test.ts | 44 +++ .../hono/test/shared/earlyPatchRoute.test.ts | 116 +++++++ .../hono/test/shared/patchAppRequest.test.ts | 134 ++++++++ packages/hono/test/shared/patchAppUse.test.ts | 19 ++ 14 files changed, 925 insertions(+), 25 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-multi-fetch.ts create mode 100644 dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts create mode 100644 packages/hono/src/shared/patchAppRequest.ts create mode 100644 packages/hono/test/shared/earlyPatchRoute.test.ts create mode 100644 packages/hono/test/shared/patchAppRequest.test.ts diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-multi-fetch.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-multi-fetch.ts new file mode 100644 index 000000000000..58eb5b504640 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-multi-fetch.ts @@ -0,0 +1,99 @@ +import { Hono } from 'hono'; +import { HTTPException } from 'hono/http-exception'; + +const ITEMS: Record = { + 'self-watering-plant': { name: 'Self-Watering Plant', stock: 5, price: 2500 }, + 'solar-powered-cyberdeck': { name: 'Solar-Powered Cyberdeck', stock: 0, price: 5000 }, +}; + +// Inventory service — standalone sub-app used as a data source. +// Mounted on the main app AND called internally via .request() from the storefront. +const inventoryApp = new Hono(); + +inventoryApp.get('/item/:productId', c => { + const productId = c.req.param('productId'); + const item = ITEMS[productId]; + if (!item) { + throw new HTTPException(404, { message: `Item ${productId} not found` }); + } + return c.json({ productId, ...item }); +}); + +inventoryApp.get('/item/:productId/stock', c => { + const productId = c.req.param('productId'); + const item = ITEMS[productId]; + if (!item) { + throw new HTTPException(404, { message: `Stock check failed: ${productId}` }); + } + return c.json({ productId, inStock: item.stock > 0, quantity: item.stock }); +}); + +// Storefront service — orchestrates internal .request() calls to inventoryApp. +const storefrontApp = new Hono(); + +storefrontApp.use('/*', async function storefrontAuth(_c, next) { + await new Promise(resolve => setTimeout(resolve, 10)); + await next(); +}); + +// Single internal fetch: look up one product +storefrontApp.get('/product/:productId', async c => { + const res = await inventoryApp.request(`/item/${c.req.param('productId')}`); + if (!res.ok) { + throw new HTTPException(404, { message: 'Product not found' }); + } + const item = await res.json(); + return c.json({ product: item, source: 'storefront' }); +}); + +// Parallel internal fetches: compare two products via Promise.all +storefrontApp.get('/compare/:productId1/:productId2', async c => { + const [res1, res2] = await Promise.all([ + inventoryApp.request(`/item/${c.req.param('productId1')}`), + inventoryApp.request(`/item/${c.req.param('productId2')}`), + ]); + if (!res1.ok || !res2.ok) { + throw new HTTPException(404, { message: 'One or more products not found' }); + } + const [item1, item2] = (await Promise.all([res1.json(), res2.json()])) as [ + Record, + Record, + ]; + return c.json({ + items: [item1, item2], + priceDifference: Math.abs(item1.price - item2.price), + }); +}); + +// Sequential chained fetches: look up item, then check its stock +storefrontApp.get('/product/:productId/availability', async c => { + const itemRes = await inventoryApp.request(`/item/${c.req.param('productId')}`); + if (!itemRes.ok) { + throw new HTTPException(404, { message: 'Product not found' }); + } + const item: Record = await itemRes.json(); + + const stockRes = await inventoryApp.request(`/item/${item.productId}/stock`); + const stock: Record = await stockRes.json(); + + return c.json({ + product: item.name, + available: stock.inStock, + quantity: stock.quantity, + }); +}); + +// Error propagation: internal 404 causes the handler to throw a plain Error +storefrontApp.get('/product-or-throw/:productId', async c => { + const res = await inventoryApp.request(`/item/${c.req.param('productId')}`); + if (!res.ok) { + throw new Error(`Failed to fetch product: ${c.req.param('productId')}`); + } + return c.json({ product: await res.json() }); +}); + +const multiFetchRoutes = new Hono(); +multiFetchRoutes.route('/inventory', inventoryApp); +multiFetchRoutes.route('/storefront', storefrontApp); + +export { multiFetchRoutes }; diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts index f1f9c3a783b3..1a3c74630648 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts @@ -3,6 +3,7 @@ import { HTTPException } from 'hono/http-exception'; import { failingMiddleware, middlewareA, middlewareB } from './middleware'; import { errorRoutes } from './route-groups/test-errors'; import { middlewareRoutes, subAppWithInlineMiddleware, subAppWithMiddleware } from './route-groups/test-middleware'; +import { multiFetchRoutes } from './route-groups/test-multi-fetch'; import { routePatterns } from './route-groups/test-route-patterns'; export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): void { @@ -48,4 +49,7 @@ export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): v // Error-specific routes: onError handler, nested sub-apps, middleware HTTPException app.route('/test-errors', errorRoutes); + + // Multi-fetch routes: storefront sub-app calls inventoryApp via .request() + app.route('/test-multi-fetch', multiFetchRoutes); } diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts new file mode 100644 index 000000000000..9e33c8d40b1c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts @@ -0,0 +1,310 @@ +import { expect, test } from '@playwright/test'; +import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; +import { APP_NAME } from './constants'; + +const STOREFRONT = '/test-multi-fetch/storefront'; +const INVENTORY = '/test-multi-fetch/inventory'; + +test.describe('multi-fetch: internal .request() calls between sub-apps', () => { + test.describe('single internal fetch', () => { + test('returns enriched product data and creates transaction with parameterized route', async ({ baseURL }) => { + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${STOREFRONT}/product/:productId` + ); + }); + + const response = await fetch(`${baseURL}${STOREFRONT}/product/self-watering-plant`); + expect(response.status).toBe(200); + + const body = await response.json(); + expect(body.product).toEqual( + expect.objectContaining({ + productId: 'self-watering-plant', + name: 'Self-Watering Plant', + stock: 5, + price: 2500, + }), + ); + expect(body.source).toBe('storefront'); + + const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${STOREFRONT}/product/:productId`); + expect(transaction.contexts?.trace?.op).toBe('http.server'); + }); + + test('creates storefrontAuth middleware span', async ({ baseURL }) => { + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${STOREFRONT}/product/:productId` + ); + }); + + const response = await fetch(`${baseURL}${STOREFRONT}/product/solar-powered-cyberdeck`); + expect(response.status).toBe(200); + + const transaction = await transactionPromise; + const spans = transaction.spans || []; + + const middlewareSpan = spans.find( + (span: { description?: string; op?: string }) => + span.op === 'middleware.hono' && span.description === 'storefrontAuth', + ); + + expect(middlewareSpan).toEqual( + expect.objectContaining({ + description: 'storefrontAuth', + op: 'middleware.hono', + origin: 'auto.middleware.hono', + }), + ); + expect(middlewareSpan?.status).not.toBe('internal_error'); + }); + }); + + test.describe('parallel internal fetches', () => { + test('aggregates data from two concurrent .request() calls', async ({ baseURL }) => { + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && + event.transaction === `GET ${STOREFRONT}/compare/:productId1/:productId2` + ); + }); + + const response = await fetch(`${baseURL}${STOREFRONT}/compare/self-watering-plant/solar-powered-cyberdeck`); + expect(response.status).toBe(200); + + const body = await response.json(); + expect(body.items).toHaveLength(2); + expect(body.items[0].productId).toBe('self-watering-plant'); + expect(body.items[1].productId).toBe('solar-powered-cyberdeck'); + expect(body.priceDifference).toBe(2500); + + const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${STOREFRONT}/compare/:productId1/:productId2`); + }); + }); + + test.describe('sequential chained fetches', () => { + test('composes data from item lookup followed by stock check', async ({ baseURL }) => { + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && + event.transaction === `GET ${STOREFRONT}/product/:productId/availability` + ); + }); + + const response = await fetch(`${baseURL}${STOREFRONT}/product/self-watering-plant/availability`); + expect(response.status).toBe(200); + + const body = await response.json(); + expect(body).toEqual({ product: 'Self-Watering Plant', available: true, quantity: 5 }); + + const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${STOREFRONT}/product/:productId/availability`); + expect(transaction.contexts?.trace?.op).toBe('http.server'); + }); + + test('reports out-of-stock item as unavailable', async ({ baseURL }) => { + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && + event.transaction === `GET ${STOREFRONT}/product/:productId/availability` + ); + }); + + const response = await fetch(`${baseURL}${STOREFRONT}/product/solar-powered-cyberdeck/availability`); + expect(response.status).toBe(200); + + const body = await response.json(); + expect(body).toEqual({ product: 'Solar-Powered Cyberdeck', available: false, quantity: 0 }); + + await transactionPromise; + }); + }); + + test.describe('error propagation from internal fetch', () => { + test('captures error when handler throws after failed internal .request()', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Failed to fetch product: nonexistent'; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && + event.transaction === `GET ${STOREFRONT}/product-or-throw/:productId` + ); + }); + + const response = await fetch(`${baseURL}${STOREFRONT}/product-or-throw/nonexistent`); + expect(response.status).toBe(500); + + const errorEvent = await errorPromise; + expect(errorEvent.exception?.values?.[0]?.value).toBe('Failed to fetch product: nonexistent'); + expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual(expect.objectContaining({ handled: false })); + expect(errorEvent.transaction).toBe(`GET ${STOREFRONT}/product-or-throw/:productId`); + + const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${STOREFRONT}/product-or-throw/:productId`); + expect(transaction.contexts?.trace?.status).toBe('internal_error'); + }); + + test('error event includes request data', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Failed to fetch product: missing'; + }); + + await fetch(`${baseURL}${STOREFRONT}/product-or-throw/missing`); + + const errorEvent = await errorPromise; + expect(errorEvent.request).toEqual( + expect.objectContaining({ + method: 'GET', + url: expect.stringContaining(`${STOREFRONT}/product-or-throw/missing`), + }), + ); + }); + }); + + test.describe('inventory sub-app direct access', () => { + test('creates its own transaction when accessed directly via HTTP', async ({ baseURL }) => { + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${INVENTORY}/item/:productId`; + }); + + const response = await fetch(`${baseURL}${INVENTORY}/item/self-watering-plant`); + expect(response.status).toBe(200); + + const body = await response.json(); + expect(body).toEqual(expect.objectContaining({ productId: 'self-watering-plant', name: 'Self-Watering Plant' })); + + const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${INVENTORY}/item/:productId`); + expect(transaction.contexts?.trace?.op).toBe('http.server'); + }); + }); + + test.describe('trace propagation through internal .request() calls', () => { + test('single internal fetch produces a hono.request child span', async ({ baseURL }) => { + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${STOREFRONT}/product/:productId` + ); + }); + + await fetch(`${baseURL}${STOREFRONT}/product/self-watering-plant`); + + const transaction = await transactionPromise; + const traceId = transaction.contexts?.trace?.trace_id; + const spans = transaction.spans || []; + + const internalRequestSpans = spans.filter((s: { op?: string }) => s.op === 'hono.request'); + + expect(internalRequestSpans).toHaveLength(1); + expect(internalRequestSpans[0]).toEqual( + expect.objectContaining({ + op: 'hono.request', + origin: 'auto.http.hono.internal_request', + trace_id: traceId, + }), + ); + expect(internalRequestSpans[0]?.description).toContain('GET /item/self-watering-plant'); + }); + + test('parallel internal fetches produce two sibling hono.request spans', async ({ baseURL }) => { + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && + event.transaction === `GET ${STOREFRONT}/compare/:productId1/:productId2` + ); + }); + + await fetch(`${baseURL}${STOREFRONT}/compare/self-watering-plant/solar-powered-cyberdeck`); + + const transaction = await transactionPromise; + const traceId = transaction.contexts?.trace?.trace_id; + const spans = transaction.spans || []; + + const internalRequestSpans = spans.filter((s: { op?: string }) => s.op === 'hono.request'); + + expect(internalRequestSpans).toHaveLength(2); + + expect(internalRequestSpans[0]?.parent_span_id).toBe(internalRequestSpans[1]?.parent_span_id); + + expect(internalRequestSpans[0]?.trace_id).toBe(traceId); + expect(internalRequestSpans[1]?.trace_id).toBe(traceId); + + expect(internalRequestSpans[0]?.origin).toBe('auto.http.hono.internal_request'); + expect(internalRequestSpans[1]?.origin).toBe('auto.http.hono.internal_request'); + }); + + test('sequential chained fetches produce two ordered hono.request spans', async ({ baseURL }) => { + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && + event.transaction === `GET ${STOREFRONT}/product/:productId/availability` + ); + }); + + await fetch(`${baseURL}${STOREFRONT}/product/self-watering-plant/availability`); + + const transaction = await transactionPromise; + const traceId = transaction.contexts?.trace?.trace_id; + const spans = transaction.spans || []; + + const internalRequestSpans = spans + .filter((s: { op?: string }) => s.op === 'hono.request') + .sort( + (a: { start_timestamp?: number }, b: { start_timestamp?: number }) => + (a.start_timestamp ?? 0) - (b.start_timestamp ?? 0), + ); + + expect(internalRequestSpans).toHaveLength(2); + + // Sequential: second span starts at or after first span ends + expect(internalRequestSpans[1].start_timestamp).toBeGreaterThanOrEqual(internalRequestSpans[0].timestamp); + + expect(internalRequestSpans[0]?.trace_id).toBe(traceId); + expect(internalRequestSpans[1]?.trace_id).toBe(traceId); + }); + + test('hono.request span has no error status for internal 4xx HTTPException', async ({ baseURL }) => { + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && + event.transaction === `GET ${STOREFRONT}/product-or-throw/:productId` + ); + }); + + await fetch(`${baseURL}${STOREFRONT}/product-or-throw/ghost`); + + const transaction = await transactionPromise; + const spans = transaction.spans || []; + + const internalRequestSpans = spans.filter((s: { op?: string }) => s.op === 'hono.request'); + + expect(internalRequestSpans).toHaveLength(1); + expect(internalRequestSpans[0]?.status).not.toBe('internal_error'); + }); + + test('error from failed internal fetch is correlated with the storefront trace', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Failed to fetch product: ghost'; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && + event.transaction === `GET ${STOREFRONT}/product-or-throw/:productId` + ); + }); + + await fetch(`${baseURL}${STOREFRONT}/product-or-throw/ghost`); + + const [errorEvent, transaction] = await Promise.all([errorPromise, transactionPromise]); + + expect(errorEvent.contexts?.trace?.trace_id).toBe(transaction.contexts?.trace?.trace_id); + expect(errorEvent.contexts?.trace?.span_id).toBeDefined(); + }); + }); +}); diff --git a/packages/hono/src/index.bun.ts b/packages/hono/src/index.bun.ts index 51fbac5fe01f..7c456a21c52e 100644 --- a/packages/hono/src/index.bun.ts +++ b/packages/hono/src/index.bun.ts @@ -1,3 +1,7 @@ +import { earlyPatchHono } from './shared/applyPatches'; + +earlyPatchHono(); + export { sentry } from './bun/middleware'; export * from '@sentry/bun'; diff --git a/packages/hono/src/index.cloudflare.ts b/packages/hono/src/index.cloudflare.ts index 99c04597a98f..e46e119c206c 100644 --- a/packages/hono/src/index.cloudflare.ts +++ b/packages/hono/src/index.cloudflare.ts @@ -1,3 +1,7 @@ +import { earlyPatchHono } from './shared/applyPatches'; + +earlyPatchHono(); + export { sentry } from './cloudflare/middleware'; export * from '@sentry/cloudflare'; diff --git a/packages/hono/src/index.node.ts b/packages/hono/src/index.node.ts index 02e94b67be89..761cd27a44a6 100644 --- a/packages/hono/src/index.node.ts +++ b/packages/hono/src/index.node.ts @@ -1,3 +1,7 @@ +import { earlyPatchHono } from './shared/applyPatches'; + +earlyPatchHono(); + export { sentry } from './node/middleware'; export * from '@sentry/node'; diff --git a/packages/hono/src/shared/applyPatches.ts b/packages/hono/src/shared/applyPatches.ts index 9ad70c1dd62a..a12484dce422 100644 --- a/packages/hono/src/shared/applyPatches.ts +++ b/packages/hono/src/shared/applyPatches.ts @@ -1,18 +1,51 @@ +import { debug } from '@sentry/core'; import type { Env, Hono } from 'hono'; +import { DEBUG_BUILD } from '../debug-build'; +import { patchAppRequest } from './patchAppRequest'; import { patchAppUse } from './patchAppUse'; -import { installRouteHookOnPrototype } from './patchRoute'; +import { type HonoRoute, type RouteHookHandle, installRouteHookOnPrototype, wrapSubAppMiddleware } from './patchRoute'; + +// Lazily set by the first call to earlyPatchHono or applyPatches. +let _routeHook: RouteHookHandle | undefined; + +/** + * Hooks `HonoBase.prototype.route` at import time, before `sentry()` runs. + * + * Collecting sub-app references early ensures nothing is missed if sub-apps are mounted synchronously before the `sentry()` middleware is registered. + */ +export function earlyPatchHono(): void { + _routeHook ??= installRouteHookOnPrototype(); +} /** * Instruments a Hono app instance for Sentry tracing in middleware and route handlers. * - * Two strategies are needed because Hono mixes instance fields and prototype methods: - * - `use` is a per-instance class field (instance own property) → must be patched on the instance - * - `route` is a prototype method → patched once globally, covers all instances + * - `use` and `request` are per-instance class fields → must be patched on the instance. + * - `route` is a prototype method → hooked once globally, covers all instances. + * - Retroactively instruments sub-apps mounted before `sentry()` was called. */ export function applyPatches(app: Hono): void { + // Always call — installRouteHookOnPrototype is idempotent and returns existing handle when prototype already patched + _routeHook = installRouteHookOnPrototype(); + // `app.use` (instance own property) — wraps middleware at registration time on this instance. patchAppUse(app); - // `route()` lives on the shared prototype and is patched once globally. - installRouteHookOnPrototype(); + patchAppRequest(app); + + _routeHook.activate(); + + const pendingSubApps = _routeHook.getPendingSubApps(); + + if (pendingSubApps.size > 0) { + DEBUG_BUILD && + debug.log(`[hono] Retroactively instrumenting ${pendingSubApps.size} sub-app(s) mounted before sentry().`); + } + + for (const subApp of pendingSubApps) { + wrapSubAppMiddleware(subApp.routes as HonoRoute[]); + patchAppRequest(subApp); + } + + pendingSubApps.clear(); } diff --git a/packages/hono/src/shared/patchAppRequest.ts b/packages/hono/src/shared/patchAppRequest.ts new file mode 100644 index 000000000000..be330466ea11 --- /dev/null +++ b/packages/hono/src/shared/patchAppRequest.ts @@ -0,0 +1,65 @@ +import { + debug, + getActiveSpan, + getOriginalFunction, + markFunctionWrapped, + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + startSpan, + type WrappedFunction, +} from '@sentry/core'; +import type { Env, Hono } from 'hono'; +import { DEBUG_BUILD } from '../debug-build'; + +const INTERNAL_REQUEST_OP = 'hono.request'; +const INTERNAL_REQUEST_ORIGIN = 'auto.http.hono.internal_request'; + +// Widened type to allow forwarding opaque args (env bindings, execution context) +type LooseRequestFn = (input: string | Request | URL, requestInit?: RequestInit, ...rest: unknown[]) => unknown; + +/** + * Patches `app.request()` on a Hono instance so that each internal dispatch + * is traced as a `hono.request` span — child of whatever span is active at + * the call site. + * + * `.request()` is a class field (arrow function), so this must run per-instance. + * Idempotent: safe to call multiple times on the same instance. + */ +export function patchAppRequest(app: Hono): void { + if (getOriginalFunction(app.request as unknown as WrappedFunction)) { + DEBUG_BUILD && debug.log('[hono] app.request already patched — skipping.'); + return; + } + + const originalRequest = app.request as LooseRequestFn; + + const patchedRequest = (input: string | Request | URL, requestInit?: RequestInit, ...rest: unknown[]) => { + if (!getActiveSpan()) { + return originalRequest(input, requestInit, ...rest); + } + + let method = requestInit?.method ?? (input instanceof Request ? input.method : 'GET'); + method = method.toUpperCase(); + + const path = + typeof input === 'string' ? input : input instanceof Request ? new URL(input.url).pathname : input.pathname; + + return startSpan( + { + name: `${method} ${path}`, + op: INTERNAL_REQUEST_OP, + onlyIfParent: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: INTERNAL_REQUEST_OP, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: INTERNAL_REQUEST_ORIGIN, + }, + }, + () => originalRequest(input, requestInit, ...rest), + ); + }; + + markFunctionWrapped(patchedRequest as unknown as WrappedFunction, originalRequest as unknown as WrappedFunction); + app.request = patchedRequest as typeof app.request; + + DEBUG_BUILD && debug.log('[hono] Patched app.request for internal dispatch tracing.'); +} diff --git a/packages/hono/src/shared/patchAppUse.ts b/packages/hono/src/shared/patchAppUse.ts index 8a6c4f46c40a..4daa84ae0c24 100644 --- a/packages/hono/src/shared/patchAppUse.ts +++ b/packages/hono/src/shared/patchAppUse.ts @@ -1,12 +1,25 @@ -import { wrapMiddlewareWithSpan } from './wrapMiddlewareSpan'; +import { debug } from '@sentry/core'; import type { Env, Hono, MiddlewareHandler } from 'hono'; +import { DEBUG_BUILD } from '../debug-build'; +import { wrapMiddlewareWithSpan } from './wrapMiddlewareSpan'; + +// oxlint-disable-next-line typescript/no-explicit-any +const patchedInstances = new WeakSet>(); /** * Patches `app.use` (instance own property) on a Hono instance to instrument middleware at registration time. * * Must be per-instance because `use` is a class field, not a prototype method. + * Idempotent. */ export function patchAppUse(app: Hono): void { + if (patchedInstances.has(app)) { + DEBUG_BUILD && debug.log('[hono] app.use already patched — skipping.'); + return; + } + + patchedInstances.add(app); + app.use = new Proxy(app.use, { apply(target: typeof app.use, thisArg: typeof app, args: Parameters): ReturnType { const [first, ...rest] = args as [unknown, ...MiddlewareHandler[]]; diff --git a/packages/hono/src/shared/patchRoute.ts b/packages/hono/src/shared/patchRoute.ts index bdc1822dad4e..d09fc56011c0 100644 --- a/packages/hono/src/shared/patchRoute.ts +++ b/packages/hono/src/shared/patchRoute.ts @@ -1,50 +1,101 @@ -import { getOriginalFunction, markFunctionWrapped } from '@sentry/core'; +import { debug, getOriginalFunction, markFunctionWrapped } from '@sentry/core'; import type { WrappedFunction } from '@sentry/core'; import type { Hono, MiddlewareHandler } from 'hono'; import { Hono as HonoClass } from 'hono'; +import { DEBUG_BUILD } from '../debug-build'; +import { patchAppRequest } from './patchAppRequest'; import { wrapMiddlewareWithSpan } from './wrapMiddlewareSpan'; -interface HonoRoute { +export type HonoRoute = { method: string; path: string; handler: MiddlewareHandler; -} +}; + +// oxlint-disable-next-line typescript/no-explicit-any +type HonoAny = Hono; + +export type RouteHookHandle = { + activate: () => void; + getPendingSubApps: () => Set; +}; + +type HonoBaseProto = { + route?: (path: string, app: HonoAny) => HonoAny; + __sentryRouteHook__?: RouteHookHandle; +}; + +/** + * Creates the two-phase state machine for the route hook. + * + * - Pre-activation: collects sub-app references into a pending set. + * - Post-activation: instruments sub-apps immediately at mount time. + */ +function createRouteHook(): { handle: RouteHookHandle; onSubAppMounted: (subApp: HonoAny) => void } { + const pendingSubApps = new Set(); + let activated = false; -interface HonoBaseProto { - // oxlint-disable-next-line typescript/no-explicit-any - route: (path: string, app: Hono) => Hono; + return { + handle: { + activate: () => { + activated = true; + }, + getPendingSubApps: () => pendingSubApps, + }, + onSubAppMounted: (subApp: HonoAny) => { + if (activated) { + DEBUG_BUILD && debug.log(`[hono] Instrumenting sub-app at mount time (${subApp.routes.length} routes).`); + wrapSubAppMiddleware(subApp.routes as HonoRoute[]); + patchAppRequest(subApp); + } else { + DEBUG_BUILD && + debug.log(`[hono] Collecting sub-app for deferred instrumentation (${subApp.routes.length} routes).`); + pendingSubApps.add(subApp); + } + }, + }; } /** - * Patches `route()` on the Hono base prototype once, globally. + * Installs a hook on `HonoBase.prototype.route` to intercept sub-app mounting. * - * Wraps sub-app middleware at mount time so that `app.route('/prefix', subApp)` is traced. - * Idempotent: safe to call multiple times. + * Returns a handle with `activate()` and `getPendingSubApps()`. + * Idempotent: subsequent calls return the same handle */ -export function installRouteHookOnPrototype(): void { - // `route` is on the base prototype, not the concrete subclass, walk up one level +export function installRouteHookOnPrototype(): RouteHookHandle { + const noopHandle: RouteHookHandle = { activate: () => {}, getPendingSubApps: () => new Set() }; + + // `route` is defined on HonoBase.prototype, one level above the concrete subclass const honoBaseProto = Object.getPrototypeOf(HonoClass.prototype) as HonoBaseProto; - if (!honoBaseProto || typeof honoBaseProto?.route !== 'function') { - return; + + if (!honoBaseProto || typeof honoBaseProto.route !== 'function') { + DEBUG_BUILD && debug.warn('[hono] Could not find HonoBase.prototype.route — sub-app instrumentation disabled.'); + return noopHandle; } - // Already patched: return + // Already patched if (getOriginalFunction(honoBaseProto.route as unknown as WrappedFunction)) { - return; + return honoBaseProto.__sentryRouteHook__ ?? noopHandle; } const originalRoute = honoBaseProto.route; + const { handle, onSubAppMounted } = createRouteHook(); - // oxlint-disable-next-line typescript/no-explicit-any - const patchedRoute = function (this: Hono, path: string, subApp: Hono): Hono { + const patchedRoute = function (this: HonoAny, path: string, subApp: HonoAny): HonoAny { if (subApp && Array.isArray(subApp.routes)) { - wrapSubAppMiddleware(subApp.routes as HonoRoute[]); + onSubAppMounted(subApp); } + return originalRoute.call(this, path, subApp); }; markFunctionWrapped(patchedRoute as unknown as WrappedFunction, originalRoute as unknown as WrappedFunction); honoBaseProto.route = patchedRoute; + honoBaseProto.__sentryRouteHook__ = handle; + + DEBUG_BUILD && debug.log('[hono] Installed route hook on HonoBase.prototype.'); + + return handle; } /** diff --git a/packages/hono/test/shared/applyPatches.test.ts b/packages/hono/test/shared/applyPatches.test.ts index f5ea5a27cbc6..03ae6e213b75 100644 --- a/packages/hono/test/shared/applyPatches.test.ts +++ b/packages/hono/test/shared/applyPatches.test.ts @@ -11,10 +11,13 @@ vi.mock('@sentry/core', async () => { setStatus: vi.fn(), end: vi.fn(), })), + startSpan: vi.fn((_opts: unknown, callback: () => unknown) => callback()), + getActiveSpan: vi.fn(() => ({ spanId: 'fake-span' })), }; }); const startInactiveSpanMock = SentryCore.startInactiveSpan as ReturnType; +const startSpanMock = SentryCore.startSpan as ReturnType; const honoBaseProto = Object.getPrototypeOf(Object.getPrototypeOf(new Hono())); const originalRoute = honoBaseProto.route; @@ -391,4 +394,45 @@ describe('applyPatches', () => { expect(startInactiveSpanMock).toHaveBeenCalledWith(expect.objectContaining({ name: 'mwForB' })); }); }); + + describe('patchAppRequest integration', () => { + it('patches .request() on sub-apps when they are mounted via route()', async () => { + const app = new Hono(); + applyPatches(app); + + const subApp = new Hono(); + subApp.get('/hello', () => new Response('world')); + + app.route('/api', subApp); + + await subApp.request('/hello'); + + expect(startSpanMock).toHaveBeenCalledTimes(1); + expect(startSpanMock).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'GET /hello', + op: 'hono.request', + attributes: expect.objectContaining({ + 'sentry.op': 'hono.request', + 'sentry.origin': 'auto.http.hono.internal_request', + }), + }), + expect.any(Function), + ); + }); + + it('does not double-patch .request() on a sub-app mounted multiple times', () => { + const app = new Hono(); + applyPatches(app); + + const subApp = new Hono(); + subApp.get('/hello', () => new Response('world')); + + app.route('/api', subApp); + const patchedRequest = subApp.request; + + app.route('/api2', subApp); + expect(subApp.request).toBe(patchedRequest); + }); + }); }); diff --git a/packages/hono/test/shared/earlyPatchRoute.test.ts b/packages/hono/test/shared/earlyPatchRoute.test.ts new file mode 100644 index 000000000000..ae54f4c50488 --- /dev/null +++ b/packages/hono/test/shared/earlyPatchRoute.test.ts @@ -0,0 +1,116 @@ +import * as SentryCore from '@sentry/core'; +import { Hono } from 'hono'; +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { applyPatches, earlyPatchHono } from '../../src/shared/applyPatches'; +import { installRouteHookOnPrototype } from '../../src/shared/patchRoute'; + +vi.mock('@sentry/core', async () => { + const actual = await vi.importActual('@sentry/core'); + return { + ...actual, + startInactiveSpan: vi.fn((_opts: unknown) => ({ + setStatus: vi.fn(), + end: vi.fn(), + })), + startSpan: vi.fn((_opts: unknown, callback: () => unknown) => callback()), + getActiveSpan: vi.fn(() => ({ spanId: 'fake-span' })), + }; +}); + +const startSpanMock = SentryCore.startSpan as ReturnType; + +const honoBaseProto = Object.getPrototypeOf(Hono.prototype) as { route: Function }; +const originalRoute = honoBaseProto.route; + +earlyPatchHono(); + +describe('earlyPatchHono (two-phase prototype hook)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterAll(() => { + honoBaseProto.route = originalRoute; + }); + + it('does NOT patch sub-app .request() at .route() time — only collects', async () => { + const subApp = new Hono(); + subApp.get('/hello', c => c.text('world')); + + const parent = new Hono(); + parent.route('/api', subApp); + + await subApp.request('/hello'); + expect(startSpanMock).not.toHaveBeenCalled(); + }); + + it('patches collected sub-apps when applyPatches activates', async () => { + const subApp = new Hono(); + subApp.get('/hello', c => c.text('world')); + + const parent = new Hono(); + parent.route('/api', subApp); + + applyPatches(parent); + + await subApp.request('/hello'); + + expect(startSpanMock).toHaveBeenCalledTimes(1); + expect(startSpanMock).toHaveBeenCalledWith( + expect.objectContaining({ name: 'GET /hello', op: 'hono.request' }), + expect.any(Function), + ); + }); + + it('preserves correct route behavior', async () => { + const subApp = new Hono(); + subApp.get('/hello', c => c.text('world')); + + const parent = new Hono(); + parent.route('/api', subApp); + + const res = await parent.fetch(new Request('http://localhost/api/hello')); + expect(res.status).toBe(200); + expect(await res.text()).toBe('world'); + }); +}); + +describe('installRouteHookOnPrototype idempotency', () => { + afterAll(() => { + honoBaseProto.route = originalRoute; + }); + + it('returns the same handle on repeated calls', () => { + const handle1 = installRouteHookOnPrototype(); + const handle2 = installRouteHookOnPrototype(); + + expect(handle1).toBe(handle2); + }); + + it('does not replace the patched route function on repeated calls', () => { + installRouteHookOnPrototype(); + const patchedRoute = honoBaseProto.route; + + installRouteHookOnPrototype(); + expect(honoBaseProto.route).toBe(patchedRoute); + }); + + it('calling activate() multiple times has no adverse effect', async () => { + const handle = installRouteHookOnPrototype(); + + handle.activate(); + handle.activate(); + handle.activate(); + + const app = new Hono(); + applyPatches(app); + + const subApp = new Hono(); + subApp.get('/hello', c => c.text('world')); + app.route('/api', subApp); + + await subApp.request('/hello'); + + expect(startSpanMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/hono/test/shared/patchAppRequest.test.ts b/packages/hono/test/shared/patchAppRequest.test.ts new file mode 100644 index 000000000000..92eb520eb208 --- /dev/null +++ b/packages/hono/test/shared/patchAppRequest.test.ts @@ -0,0 +1,134 @@ +import * as SentryCore from '@sentry/core'; +import { Hono } from 'hono'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { patchAppRequest } from '../../src/shared/patchAppRequest'; + +vi.mock('@sentry/core', async () => { + const actual = await vi.importActual('@sentry/core'); + return { + ...actual, + startSpan: vi.fn((_opts: unknown, callback: () => unknown) => callback()), + getActiveSpan: vi.fn(() => ({ spanId: 'fake-span' })), + }; +}); + +const startSpanMock = SentryCore.startSpan as ReturnType; +const getActiveSpanMock = SentryCore.getActiveSpan as ReturnType; + +describe('patchAppRequest', () => { + beforeEach(() => { + vi.clearAllMocks(); + getActiveSpanMock.mockReturnValue({ spanId: 'fake-span' }); + }); + + it('creates a hono.request span when .request() is called with an active parent span', async () => { + const app = new Hono(); + app.get('/hello', c => c.text('world')); + patchAppRequest(app); + + await app.request('/hello'); + + expect(startSpanMock).toHaveBeenCalledTimes(1); + expect(startSpanMock).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'GET /hello', + op: 'hono.request', + onlyIfParent: true, + attributes: expect.objectContaining({ + 'sentry.op': 'hono.request', + 'sentry.origin': 'auto.http.hono.internal_request', + }), + }), + expect.any(Function), + ); + }); + + it('skips span creation when there is no active span', async () => { + getActiveSpanMock.mockReturnValue(undefined); + + const app = new Hono(); + app.get('/hello', c => c.text('world')); + patchAppRequest(app); + + const res = await app.request('/hello'); + + expect(startSpanMock).not.toHaveBeenCalled(); + expect(await res.text()).toBe('world'); + }); + + it('uses the method from requestInit when provided', async () => { + const app = new Hono(); + app.post('/submit', c => c.text('ok')); + patchAppRequest(app); + + await app.request('/submit', { method: 'POST' }); + + expect(startSpanMock).toHaveBeenCalledWith(expect.objectContaining({ name: 'POST /submit' }), expect.any(Function)); + }); + + it('uses the method from a Request object when no requestInit is provided', async () => { + const app = new Hono(); + app.post('/submit', c => c.text('ok')); + patchAppRequest(app); + + await app.request(new Request('http://localhost/submit', { method: 'POST' })); + + expect(startSpanMock).toHaveBeenCalledWith(expect.objectContaining({ name: 'POST /submit' }), expect.any(Function)); + }); + + it('defaults to GET when no method info is available', async () => { + const app = new Hono(); + app.get('/hello', c => c.text('world')); + patchAppRequest(app); + + await app.request('/hello'); + + expect(startSpanMock).toHaveBeenCalledWith(expect.objectContaining({ name: 'GET /hello' }), expect.any(Function)); + }); + + it('does not double-patch when called twice on the same instance', async () => { + const app = new Hono(); + app.get('/hello', c => c.text('world')); + + patchAppRequest(app); + const firstPatched = app.request; + + patchAppRequest(app); + expect(app.request).toBe(firstPatched); + }); + + it('preserves the original .request() return value', async () => { + const app = new Hono(); + app.get('/hello', c => c.json({ message: 'world' })); + patchAppRequest(app); + + const res = await app.request('/hello'); + expect(res.status).toBe(200); + + const body = await res.json(); + expect(body).toEqual({ message: 'world' }); + }); + + it('stores the original request via __sentry_original__', () => { + const app = new Hono(); + const originalRequest = app.request; + patchAppRequest(app); + + // oxlint-disable-next-line typescript/no-explicit-any + const sentryOriginal = (app.request as any).__sentry_original__; + expect(sentryOriginal).toBe(originalRequest); + }); + + it('extracts pathname from a Request object input', async () => { + const app = new Hono(); + app.get('/items/abc', c => c.text('found')); + patchAppRequest(app); + + await app.request(new Request('http://localhost/items/abc')); + + expect(startSpanMock).toHaveBeenCalledWith( + expect.objectContaining({ name: 'GET /items/abc' }), + expect.any(Function), + ); + }); +}); diff --git a/packages/hono/test/shared/patchAppUse.test.ts b/packages/hono/test/shared/patchAppUse.test.ts index a65c9c526a83..a82261eb3610 100644 --- a/packages/hono/test/shared/patchAppUse.test.ts +++ b/packages/hono/test/shared/patchAppUse.test.ts @@ -135,6 +135,25 @@ describe('patchAppUse (middleware spans)', () => { expect(firstCall![0].name).not.toBe(secondCall![0].name); }); + it('does not stack proxies when called twice on the same instance', () => { + const app = new Hono(); + patchAppUse(app); + const firstUse = app.use; + + patchAppUse(app); + expect(app.use).toBe(firstUse); + }); + + it('patches distinct instances independently', () => { + const app1 = new Hono(); + const app2 = new Hono(); + + patchAppUse(app1); + patchAppUse(app2); + + expect(app1.use).not.toBe(app2.use); + }); + it('preserves this context when calling the original use (Proxy forwards thisArg)', () => { type FakeApp = { _capturedThis: unknown; From 40aed03a61788729eb6a55bde0e29b2c250b6742 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 12 May 2026 17:24:25 +0200 Subject: [PATCH 2/4] chore(hono): Add missing debug-build.ts for conditional logging Co-Authored-By: Claude Opus 4.6 Co-authored-by: Cursor --- packages/hono/src/debug-build.ts | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 packages/hono/src/debug-build.ts diff --git a/packages/hono/src/debug-build.ts b/packages/hono/src/debug-build.ts new file mode 100644 index 000000000000..60aa50940582 --- /dev/null +++ b/packages/hono/src/debug-build.ts @@ -0,0 +1,8 @@ +declare const __DEBUG_BUILD__: boolean; + +/** + * This serves as a build time flag that will be true by default, but false in non-debug builds or if users replace `__SENTRY_DEBUG__` in their generated code. + * + * ATTENTION: This constant must never cross package boundaries (i.e. be exported) to guarantee that it can be used for tree shaking. + */ +export const DEBUG_BUILD = __DEBUG_BUILD__; From 29b0f0fcc6c58aa02c73047aaa7f6687d4c58df4 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 12 May 2026 17:46:27 +0200 Subject: [PATCH 3/4] add time tolerance --- .../test-applications/hono-4/tests/multi-fetch.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts index 9e33c8d40b1c..3fde49185424 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts @@ -261,8 +261,10 @@ test.describe('multi-fetch: internal .request() calls between sub-apps', () => { expect(internalRequestSpans).toHaveLength(2); - // Sequential: second span starts at or after first span ends - expect(internalRequestSpans[1].start_timestamp).toBeGreaterThanOrEqual(internalRequestSpans[0].timestamp); + // Sequential: second span starts at or after first span ends (with tolerance for clock precision) + expect(internalRequestSpans[1].start_timestamp).toBeGreaterThanOrEqual( + internalRequestSpans[0].timestamp! - 0.001, + ); expect(internalRequestSpans[0]?.trace_id).toBe(traceId); expect(internalRequestSpans[1]?.trace_id).toBe(traceId); From 72e8c2e97ce5cf756ce7bcd211e7bea692c5a262 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Wed, 13 May 2026 09:58:53 +0200 Subject: [PATCH 4/4] extract pathname without hostname --- packages/hono/src/shared/patchAppRequest.ts | 11 +++++++-- .../hono/test/shared/patchAppRequest.test.ts | 23 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/hono/src/shared/patchAppRequest.ts b/packages/hono/src/shared/patchAppRequest.ts index be330466ea11..38122cfa3066 100644 --- a/packages/hono/src/shared/patchAppRequest.ts +++ b/packages/hono/src/shared/patchAppRequest.ts @@ -17,6 +17,14 @@ const INTERNAL_REQUEST_ORIGIN = 'auto.http.hono.internal_request'; // Widened type to allow forwarding opaque args (env bindings, execution context) type LooseRequestFn = (input: string | Request | URL, requestInit?: RequestInit, ...rest: unknown[]) => unknown; +function extractPathname(input: string | Request | URL): string { + if (typeof input === 'string') { + return /^https?:\/\//.test(input) ? new URL(input).pathname : input; + } + + return input instanceof Request ? new URL(input.url).pathname : input.pathname; +} + /** * Patches `app.request()` on a Hono instance so that each internal dispatch * is traced as a `hono.request` span — child of whatever span is active at @@ -41,8 +49,7 @@ export function patchAppRequest(app: Hono): void { let method = requestInit?.method ?? (input instanceof Request ? input.method : 'GET'); method = method.toUpperCase(); - const path = - typeof input === 'string' ? input : input instanceof Request ? new URL(input.url).pathname : input.pathname; + const path = extractPathname(input); return startSpan( { diff --git a/packages/hono/test/shared/patchAppRequest.test.ts b/packages/hono/test/shared/patchAppRequest.test.ts index 92eb520eb208..1c35f21a3b23 100644 --- a/packages/hono/test/shared/patchAppRequest.test.ts +++ b/packages/hono/test/shared/patchAppRequest.test.ts @@ -119,6 +119,29 @@ describe('patchAppRequest', () => { expect(sentryOriginal).toBe(originalRequest); }); + it('extracts pathname from a full URL string instead of using the raw string', async () => { + const app = new Hono(); + app.get('/api/hello', c => c.text('world')); + patchAppRequest(app); + + await app.request('http://localhost/api/hello'); + + expect(startSpanMock).toHaveBeenCalledWith( + expect.objectContaining({ name: 'GET /api/hello' }), + expect.any(Function), + ); + }); + + it('extracts pathname from an https URL string', async () => { + const app = new Hono(); + app.get('/secure', c => c.text('ok')); + patchAppRequest(app); + + await app.request('https://example.com/secure'); + + expect(startSpanMock).toHaveBeenCalledWith(expect.objectContaining({ name: 'GET /secure' }), expect.any(Function)); + }); + it('extracts pathname from a Request object input', async () => { const app = new Hono(); app.get('/items/abc', c => c.text('found'));