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 53519a1bd80c..ba07bb7db4ca 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/package.json +++ b/dev-packages/e2e-tests/test-applications/hono-4/package.json @@ -5,7 +5,7 @@ "private": true, "scripts": { "dev:cf": "wrangler dev --var \"E2E_TEST_DSN:$E2E_TEST_DSN\" --log-level=$(test $CI && echo 'none' || echo 'log')", - "dev:node": "node --import tsx/esm --import @sentry/node/preload src/entry.node.ts", + "dev:node": "node --import tsx/esm --import ./src/instrument.node.ts src/entry.node.ts", "dev:bun": "bun src/entry.bun.ts", "build": "wrangler deploy --dry-run", "test:build": "pnpm install && pnpm build", diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/entry.node.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/entry.node.ts index eb2c669c6806..898a92e08be4 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/src/entry.node.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/entry.node.ts @@ -3,17 +3,9 @@ import { sentry } from '@sentry/hono/node'; import { serve } from '@hono/node-server'; import { addRoutes } from './routes'; -const app = new Hono<{ Bindings: { E2E_TEST_DSN: string } }>(); +const app = new Hono(); -app.use( - // @ts-expect-error - Env is not yet in type - sentry(app, { - dsn: process.env.E2E_TEST_DSN, - environment: 'qa', - tracesSampleRate: 1.0, - tunnel: 'http://localhost:3031/', - }), -); +app.use(sentry(app)); addRoutes(app); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/instrument.node.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/instrument.node.ts new file mode 100644 index 000000000000..82f2a3864125 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/instrument.node.ts @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/hono/node'; + +Sentry.init({ + dsn: process.env.E2E_TEST_DSN, + environment: 'qa', + tracesSampleRate: 1.0, + tunnel: 'http://localhost:3031/', +}); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/constants.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/constants.ts index 74905baee532..67fb8f7c9fda 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/constants.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/constants.ts @@ -1,6 +1,5 @@ export type Runtime = 'cloudflare' | 'node' | 'bun'; -export const RUNTIME = (process.env.RUNTIME || 'cloudflare') as Runtime; -export const isNode = RUNTIME === 'node'; +export const RUNTIME = (process.env.RUNTIME || 'node') as Runtime; export const APP_NAME = 'hono-4'; diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts index 967d639baa35..e3b4556bfd18 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts @@ -1,27 +1,20 @@ import { expect, test } from '@playwright/test'; import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; import { type SpanJSON } from '@sentry/core'; -import { APP_NAME, isNode } from './constants'; - -// In Node, @sentry/node/preload eagerly activates the OTel HonoInstrumentation, -// which wraps all Hono instance methods at construction time via WrappedHono. -const MIDDLEWARE_ORIGIN = 'auto.middleware.hono'; -const OTEL_ORIGIN = 'auto.http.otel.hono'; +import { APP_NAME } from './constants'; const SCENARIOS = [ { name: 'root app middleware', prefix: '/test-middleware', - origin: MIDDLEWARE_ORIGIN, }, { name: 'sub-app middleware (route group)', prefix: '/test-subapp-middleware', - origin: isNode ? OTEL_ORIGIN : MIDDLEWARE_ORIGIN, }, ] as const; -for (const { name, prefix, origin } of SCENARIOS) { +for (const { name, prefix } of SCENARIOS) { test.describe(name, () => { test('creates a span for named middleware', async ({ baseURL }) => { const transactionPromise = waitForTransaction(APP_NAME, event => { @@ -43,7 +36,7 @@ for (const { name, prefix, origin } of SCENARIOS) { expect.objectContaining({ description: 'middlewareA', op: 'middleware.hono', - origin, + origin: 'auto.middleware.hono', status: 'ok', }), ); @@ -68,18 +61,13 @@ for (const { name, prefix, origin } of SCENARIOS) { expect.objectContaining({ description: '', op: 'middleware.hono', - origin: MIDDLEWARE_ORIGIN, + origin: 'auto.middleware.hono', status: 'ok', }), ); }); test('multiple middleware are sibling spans under the same parent', async ({ baseURL }) => { - test.skip( - isNode, - 'Node double-instruments middleware (too many spans) - TODO: fix this in the SDK and re-enable the test', - ); - const transactionPromise = waitForTransaction(APP_NAME, event => { return event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${prefix}/multi`; }); @@ -90,7 +78,6 @@ for (const { name, prefix, origin } of SCENARIOS) { const transaction = await transactionPromise; const spans = transaction.spans || []; - // Sort spans because they are in a different order in Node/Bun (OTel-based) const middlewareSpans = spans.sort((a, b) => (a.start_timestamp ?? 0) - (b.start_timestamp ?? 0)); expect(middlewareSpans).toHaveLength(2); @@ -139,10 +126,6 @@ for (const { name, prefix, origin } of SCENARIOS) { const transaction = await transactionPromise; const spans = transaction.spans || []; - // On the /error path only one middleware (failingMiddleware) is registered, - // so we can find the error span by status alone. On Node for sub-apps, the - // OTel layer wraps before patchRoute, so the function name may be lost in - // the patchRoute span — but the error status is always set. const failingSpan = spans.find( (span: SpanJSON) => span.op === 'middleware.hono' && span.status === 'internal_error', ); @@ -169,36 +152,8 @@ for (const { name, prefix, origin } of SCENARIOS) { }); } -test.describe('.all() handler on sub-app (method ALL edge case)', () => { - test('Node: OTel wraps .all() and produces a hono span', async ({ baseURL }) => { - test.skip(!isNode, 'Node-specific: OTel wraps .all() at construction time'); - - const transactionPromise = waitForTransaction(APP_NAME, event => { - return ( - event.contexts?.trace?.op === 'http.server' && event.transaction === 'GET /test-subapp-middleware/all-handler' - ); - }); - - const response = await fetch(`${baseURL}/test-subapp-middleware/all-handler`); - expect(response.status).toBe(200); - - const body = await response.json(); - expect(body).toEqual({ handler: 'all' }); - - const transaction = await transactionPromise; - const spans = transaction.spans || []; - - // On Node, OTel wraps .all() at construction time. Since the handler - // returns a Response, OTel classifies it as 'request_handler' (not - // middleware). patchRoute also wraps it but sees the anonymous OTel wrapper. - // Either way, the handler IS instrumented — verify any hono span exists. - const honoSpan = spans.find((span: SpanJSON) => span.op?.endsWith('.hono')); - expect(honoSpan).toBeDefined(); - }); - - test('Bun/Cloudflare: patchRoute wraps .all() as middleware span', async ({ baseURL }) => { - test.skip(isNode, 'Bun/Cloudflare-specific: patchRoute is the sole wrapper'); - +test.describe('patchRoute wraps .all() as middleware span (in sub-app)', () => { + test('patchRoute wraps .all() as middleware span', async ({ baseURL }) => { const transactionPromise = waitForTransaction(APP_NAME, event => { return ( event.contexts?.trace?.op === 'http.server' && event.transaction === 'GET /test-subapp-middleware/all-handler' @@ -225,7 +180,7 @@ test.describe('.all() handler on sub-app (method ALL edge case)', () => { expect.objectContaining({ description: 'allCatchAll', op: 'middleware.hono', - origin: MIDDLEWARE_ORIGIN, + origin: 'auto.middleware.hono', status: 'ok', }), ); diff --git a/packages/hono/README.md b/packages/hono/README.md index 236a4133bf67..5b16e9d251d9 100644 --- a/packages/hono/README.md +++ b/packages/hono/README.md @@ -33,7 +33,7 @@ npm install @sentry/hono Additionally to `@sentry/hono`, install the `@sentry/cloudflare` package: -```bashbash +```bash npm install --save @sentry/cloudflare ``` @@ -100,54 +100,60 @@ export default app; Additionally to `@sentry/hono`, install the `@sentry/node` package: -```bashbash +```bash npm install --save @sentry/node ``` Make sure the installed version always stays in sync. The `@sentry/node` package is a required peer dependency when using `@sentry/hono/node`. You won't import `@sentry/node` directly in your code, but it needs to be installed in your project. -### 2. Initialize Sentry in your Hono app +### 2. Initialize Sentry in a separate file -Initialize the Sentry Hono middleware as early as possible in your app: +Create an `instrument.mjs` (or `instrument.ts`) file that initializes Sentry before the rest of your application runs. +This ensures Sentry can wrap third-party libraries (e.g. database clients) as early as possible: ```ts -import { Hono } from 'hono'; -import { serve } from '@hono/node-server'; -import { sentry } from '@sentry/hono/node'; - -const app = new Hono(); - -// Initialize Sentry middleware right after creating the app -app.use( - sentry(app, { - dsn: '__DSN__', // or process.env.SENTRY_DSN - tracesSampleRate: 1.0, - }), -); - -// ... your routes and other middleware +// instrument.mjs (or instrument.ts) +import * as Sentry from '@sentry/hono/node'; -serve(app); +Sentry.init({ + dsn: '__DSN__', + tracesSampleRate: 1.0, +}); ``` -### 3. Add `preload` script to start command - -To ensure that Sentry can capture spans from third-party libraries (e.g. database clients) used in your Hono app, Sentry needs to wrap these libraries as early as possible. +### 3. Load the instrument file with `--import` -When starting the Hono Node application, use the `@sentry/node/preload` hook with the `--import` CLI option to ensure modules are wrapped before the application code runs: +When starting your Hono Node application, use the `--import` CLI flag to load `instrument.mjs` before your app code: ```bash -node --import @sentry/node/preload index.js +node --import ./instrument.mjs app.js ``` This option can also be added to the `NODE_OPTIONS` environment variable: ```bash -NODE_OPTIONS="--import @sentry/node/preload" +NODE_OPTIONS="--import ./instrument.mjs" ``` -Read more about this preload script in the docs: https://docs.sentry.io/platforms/javascript/guides/hono/install/late-initialization/#late-initialization-with-esm +### 4. Add the Sentry middleware to your Hono app + +Add the `sentry` middleware to your Hono app. Since Sentry was already initialized in the instrument file, no options are needed: + +```ts +import { Hono } from 'hono'; +import { serve } from '@hono/node-server'; +import { sentry } from '@sentry/hono/node'; + +const app = new Hono(); + +// Add Sentry middleware right after creating the app +app.use(sentry(app)); + +// ... your routes and other middleware + +serve(app); +``` ## Setup (Bun) @@ -155,7 +161,7 @@ Read more about this preload script in the docs: https://docs.sentry.io/platform Additionally to `@sentry/hono`, install the `@sentry/bun` package: -```bashbash +```bash npm install --save @sentry/bun ``` diff --git a/packages/hono/src/node/middleware.ts b/packages/hono/src/node/middleware.ts index 07d3c4ed2fa8..b822882619a0 100644 --- a/packages/hono/src/node/middleware.ts +++ b/packages/hono/src/node/middleware.ts @@ -8,13 +8,16 @@ export interface HonoNodeOptions extends Options {} /** * Sentry middleware for Hono running in a Node runtime environment. + * + * @param app The root Hono application instance to which the middleware will be applied. + * @param options Optional Sentry initialization options, which **should usually be omitted** when Sentry is initialized externally (e.g. in an `instrument.ts` file loaded via `--import`). + * If provided, the middleware will initialize Sentry internally using these options. If omitted, the middleware assumes Sentry has already been initialized externally. */ -export const sentry = (app: Hono, options: HonoNodeOptions): MiddlewareHandler => { - const isDebug = options.debug; - - isDebug && debug.log('Initialized Sentry Hono middleware (Node)'); - - init(options); +export const sentry = (app: Hono, options?: HonoNodeOptions): MiddlewareHandler => { + if (options) { + options.debug && debug.log('Initialized Sentry Hono middleware (Node)'); + init(options); + } applyPatches(app); diff --git a/packages/hono/src/node/sdk.ts b/packages/hono/src/node/sdk.ts index 936cf612bb44..0ce1f5bebfec 100644 --- a/packages/hono/src/node/sdk.ts +++ b/packages/hono/src/node/sdk.ts @@ -1,5 +1,5 @@ import type { Client } from '@sentry/core'; -import { applySdkMetadata } from '@sentry/core'; +import { applySdkMetadata, debug, getClient } from '@sentry/core'; import { init as initNode } from '@sentry/node'; import type { HonoNodeOptions } from './middleware'; import { buildFilteredIntegrations } from '../shared/buildFilteredIntegrations'; @@ -7,14 +7,17 @@ import { buildFilteredIntegrations } from '../shared/buildFilteredIntegrations'; /** * Initializes Sentry for Hono running in a Node runtime environment. * - * In general, it is recommended to initialize Sentry via the `sentry()` middleware, as it sets up everything by default and calls `init` internally. - * - * When manually calling `init`, add the `honoIntegration` to the `integrations` array to set up the Hono integration. + * This function should be called in an `instrument.ts` file loaded via `--import` to set up Sentry globally for the application. */ export function init(options: HonoNodeOptions): Client | undefined { + const existingClient = getClient(); + if (existingClient) { + debug.log('Sentry is already initialized, skipping re-initialization.'); + return existingClient; + } + applySdkMetadata(options, 'hono', ['hono', 'node']); - // Remove Hono from the SDK defaults to prevent double instrumentation: @sentry/node const filteredOptions: HonoNodeOptions = { ...options, integrations: buildFilteredIntegrations(options.integrations, false), diff --git a/packages/hono/test/node/middleware.test.ts b/packages/hono/test/node/middleware.test.ts index b6561098ed8a..ab1763474256 100644 --- a/packages/hono/test/node/middleware.test.ts +++ b/packages/hono/test/node/middleware.test.ts @@ -3,6 +3,7 @@ import { SDK_VERSION } from '@sentry/core'; import { Hono } from 'hono'; import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest'; import { sentry } from '../../src/node/middleware'; +import { init } from '../../src/node/sdk'; vi.mock('@sentry/node', () => ({ init: vi.fn(), @@ -28,7 +29,7 @@ describe('Hono Node Middleware', () => { vi.clearAllMocks(); }); - describe('sentry middleware', () => { + describe('sentry middleware with options (inline init)', () => { it('calls applySdkMetadata with "hono"', () => { const app = new Hono(); const options = { @@ -94,26 +95,6 @@ describe('Hono Node Middleware', () => { ); }); - it('returns a middleware handler function', () => { - const app = new Hono(); - const options = { - dsn: 'https://public@dsn.ingest.sentry.io/1337', - }; - - const middleware = sentry(app, options); - - expect(middleware).toBeDefined(); - expect(typeof middleware).toBe('function'); - expect(middleware).toHaveLength(2); // Hono middleware takes (context, next) - }); - - it('returns an async middleware handler', () => { - const app = new Hono(); - const middleware = sentry(app, {}); - - expect(middleware.constructor.name).toBe('AsyncFunction'); - }); - it('passes an integrations function to initNode (never a raw array)', () => { const app = new Hono(); sentry(app, { dsn: 'https://public@dsn.ingest.sentry.io/1337' }); @@ -146,4 +127,75 @@ describe('Hono Node Middleware', () => { ); }); }); + + describe('sentry middleware without options (external init)', () => { + it('does not call init when no options are provided', () => { + const app = new Hono(); + sentry(app); + + expect(initNodeMock).not.toHaveBeenCalled(); + expect(applySdkMetadataMock).not.toHaveBeenCalled(); + }); + + it('returns a middleware handler function', () => { + const app = new Hono(); + const middleware = sentry(app); + + expect(middleware).toBeDefined(); + expect(typeof middleware).toBe('function'); + expect(middleware).toHaveLength(2); + }); + + it('returns an async middleware handler', () => { + const app = new Hono(); + const middleware = sentry(app); + + expect(middleware.constructor.name).toBe('AsyncFunction'); + }); + }); + + describe('common behavior', () => { + it('returns a middleware handler function when options are provided', () => { + const app = new Hono(); + const middleware = sentry(app, { dsn: 'https://public@dsn.ingest.sentry.io/1337' }); + + expect(middleware).toBeDefined(); + expect(typeof middleware).toBe('function'); + expect(middleware).toHaveLength(2); + }); + + it('returns an async middleware handler when options are provided', () => { + const app = new Hono(); + const middleware = sentry(app, {}); + + expect(middleware.constructor.name).toBe('AsyncFunction'); + }); + }); + + describe('double-init guard', () => { + it('skips re-initialization when a client already exists', () => { + const fakeClient = { getOptions: () => ({}) }; + const getClientSpy = vi + .spyOn(SentryCore, 'getClient') + .mockReturnValue(fakeClient as unknown as SentryCore.Client); + + const result = init({ dsn: 'https://public@dsn.ingest.sentry.io/1337' }); + + expect(result).toBe(fakeClient); + expect(initNodeMock).not.toHaveBeenCalled(); + expect(applySdkMetadataMock).not.toHaveBeenCalled(); + + getClientSpy.mockRestore(); + }); + + it('initializes normally when no client exists yet', () => { + const getClientSpy = vi.spyOn(SentryCore, 'getClient').mockReturnValue(undefined); + + init({ dsn: 'https://public@dsn.ingest.sentry.io/1337' }); + + expect(initNodeMock).toHaveBeenCalledTimes(1); + + getClientSpy.mockRestore(); + }); + }); });