review comments

Author: s1gr1d

packages/hono/README.md

@@ -138,7 +138,7 @@ NODE_OPTIONS="--import ./instrument.mjs"
 
 ### 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:
+Add the `sentry` middleware to your Hono app. Since Sentry was already initialized in the instrument file, no options are passed here:
 
 ```ts
 import { Hono } from 'hono';

packages/hono/src/node/middleware.ts

@@ -1,4 +1,4 @@
-import { type BaseTransportOptions, debug, type Options } from '@sentry/core';
+import { type BaseTransportOptions, debug, type Options, getClient } from '@sentry/core';
 import { init } from './sdk';
 import type { Hono, MiddlewareHandler } from 'hono';
 import { requestHandler, responseHandler } from '../shared/middlewareHandlers';
@@ -7,16 +7,22 @@ import { applyPatches } from '../shared/applyPatches';
 export interface HonoNodeOptions extends Options<BaseTransportOptions> {}
 
 /**
- * Sentry middleware for Hono running in a Node runtime environment.
+ * Sentry middleware for Hono applications running in a Node.js 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.
+ * This middleware enhances your Hono application by automatically instrumenting incoming requests and outgoing responses.
+ * It also applies the necessary patches to ensure Sentry captures execution context correctly in Node.js.
+ *
+ * **Note:** You must initialize Sentry separately before using this middleware. Typically, this is done by calling `Sentry.init()` in an `instrument.ts` file and loading it via the Node `--import` flag.
  */
-export const sentry = (app: Hono, options?: HonoNodeOptions): MiddlewareHandler => {
-  if (options) {
-    options.debug && debug.log('Initialized Sentry Hono middleware (Node)');
-    init(options);
+export const sentry = (app: Hono): MiddlewareHandler => {
+  const sentryClient = getClient();
+  if (sentryClient === undefined) {
+    debug.warn(
+      'Sentry is not initialized. Call `init()` from @sentry/hono/node in an `instrument.ts` file loaded via `--import` to set up Sentry for your application.',
+    );
+  } else {
+    sentryClient.getOptions().debug &&
+      debug.log('Sentry is initialized, proceeding to set up Hono `sentry` middleware.');
   }
 
   applyPatches(app);

packages/hono/src/node/sdk.ts

@@ -12,7 +12,7 @@ import { buildFilteredIntegrations } from '../shared/buildFilteredIntegrations';
 export function init(options: HonoNodeOptions): Client | undefined {
   const existingClient = getClient();
   if (existingClient) {
-    debug.log('Sentry is already initialized, skipping re-initialization.');
+    existingClient.getOptions().debug && debug.log('Sentry is already initialized, skipping re-initialization.');
     return existingClient;
   }
 

packages/hono/test/node/middleware.test.ts

@@ -1,7 +1,6 @@
 import * as SentryCore from '@sentry/core';
-import { SDK_VERSION } from '@sentry/core';
 import { Hono } from 'hono';
-import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
 import { sentry } from '../../src/node/middleware';
 import { init } from '../../src/node/sdk';
 
@@ -12,129 +11,17 @@ vi.mock('@sentry/node', () => ({
 // eslint-disable-next-line @typescript-eslint/consistent-type-imports
 const { init: initNodeMock } = await vi.importMock<typeof import('@sentry/node')>('@sentry/node');
 
-vi.mock('@sentry/core', async () => {
-  const actual = await vi.importActual('@sentry/core');
-  return {
-    ...actual,
-    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-    // @ts-ignore
-    applySdkMetadata: vi.fn(actual.applySdkMetadata),
-  };
-});
-
-const applySdkMetadataMock = SentryCore.applySdkMetadata as Mock;
-
 describe('Hono Node Middleware', () => {
   beforeEach(() => {
     vi.clearAllMocks();
   });
 
-  describe('sentry middleware with options (inline init)', () => {
-    it('calls applySdkMetadata with "hono"', () => {
-      const app = new Hono();
-      const options = {
-        dsn: 'https://public@dsn.ingest.sentry.io/1337',
-      };
-
-      sentry(app, options);
-
-      expect(applySdkMetadataMock).toHaveBeenCalledTimes(1);
-      expect(applySdkMetadataMock).toHaveBeenCalledWith(options, 'hono', ['hono', 'node']);
-    });
-
-    it('calls init from @sentry/node', () => {
-      const app = new Hono();
-      const options = {
-        dsn: 'https://public@dsn.ingest.sentry.io/1337',
-      };
-
-      sentry(app, options);
-
-      expect(initNodeMock).toHaveBeenCalledTimes(1);
-      expect(initNodeMock).toHaveBeenCalledWith(
-        expect.objectContaining({
-          dsn: 'https://public@dsn.ingest.sentry.io/1337',
-        }),
-      );
-    });
-
-    it('sets SDK metadata before calling Node init', () => {
-      const app = new Hono();
-      const options = {
-        dsn: 'https://public@dsn.ingest.sentry.io/1337',
-      };
-
-      sentry(app, options);
-
-      const applySdkMetadataCallOrder = applySdkMetadataMock.mock.invocationCallOrder[0];
-      const initNodeCallOrder = (initNodeMock as Mock).mock.invocationCallOrder[0];
-
-      expect(applySdkMetadataCallOrder).toBeLessThan(initNodeCallOrder as number);
-    });
-
-    it('preserves all user options', () => {
-      const app = new Hono();
-      const options = {
-        dsn: 'https://public@dsn.ingest.sentry.io/1337',
-        environment: 'production',
-        sampleRate: 0.5,
-        tracesSampleRate: 1.0,
-        debug: true,
-      };
-
-      sentry(app, options);
-
-      expect(initNodeMock).toHaveBeenCalledWith(
-        expect.objectContaining({
-          dsn: 'https://public@dsn.ingest.sentry.io/1337',
-          environment: 'production',
-          sampleRate: 0.5,
-          tracesSampleRate: 1.0,
-          debug: true,
-        }),
-      );
-    });
-
-    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' });
-
-      const callArgs = (initNodeMock as Mock).mock.calls[0]?.[0];
-      expect(typeof callArgs.integrations).toBe('function');
-    });
-
-    it('includes hono SDK metadata', () => {
-      const app = new Hono();
-      const options = {
-        dsn: 'https://public@dsn.ingest.sentry.io/1337',
-      };
-
-      sentry(app, options);
-
-      expect(initNodeMock).toHaveBeenCalledWith(
-        expect.objectContaining({
-          _metadata: expect.objectContaining({
-            sdk: expect.objectContaining({
-              name: 'sentry.javascript.hono',
-              version: SDK_VERSION,
-              packages: [
-                { name: 'npm:@sentry/hono', version: SDK_VERSION },
-                { name: 'npm:@sentry/node', version: SDK_VERSION },
-              ],
-            }),
-          }),
-        }),
-      );
-    });
-  });
-
-  describe('sentry middleware without options (external init)', () => {
-    it('does not call init when no options are provided', () => {
+  describe('sentry middleware (external init)', () => {
+    it('does not call init', () => {
       const app = new Hono();
       sentry(app);
 
       expect(initNodeMock).not.toHaveBeenCalled();
-      expect(applySdkMetadataMock).not.toHaveBeenCalled();
     });
 
     it('returns a middleware handler function', () => {
@@ -152,23 +39,26 @@ describe('Hono Node Middleware', () => {
 
       expect(middleware.constructor.name).toBe('AsyncFunction');
     });
-  });
 
-  describe('common behavior', () => {
-    it('returns a middleware handler function when options are provided', () => {
+    it('emits a warning when Sentry is not initialized', () => {
+      const warnSpy = vi.spyOn(SentryCore.debug, 'warn');
+      vi.spyOn(SentryCore, 'getClient').mockReturnValue(undefined);
+
       const app = new Hono();
-      const middleware = sentry(app, { dsn: 'https://public@dsn.ingest.sentry.io/1337' });
+      sentry(app);
 
-      expect(middleware).toBeDefined();
-      expect(typeof middleware).toBe('function');
-      expect(middleware).toHaveLength(2);
+      expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Sentry is not initialized'));
     });
 
-    it('returns an async middleware handler when options are provided', () => {
+    it('does not emit a warning when Sentry is already initialized', () => {
+      const warnSpy = vi.spyOn(SentryCore.debug, 'warn');
+      const fakeClient = { getOptions: () => ({ debug: false }) };
+      vi.spyOn(SentryCore, 'getClient').mockReturnValue(fakeClient as unknown as SentryCore.Client);
+
       const app = new Hono();
-      const middleware = sentry(app, {});
+      sentry(app);
 
-      expect(middleware.constructor.name).toBe('AsyncFunction');
+      expect(warnSpy).not.toHaveBeenCalled();
     });
   });
 
@@ -183,7 +73,6 @@ describe('Hono Node Middleware', () => {
 
       expect(result).toBe(fakeClient);
       expect(initNodeMock).not.toHaveBeenCalled();
-      expect(applySdkMetadataMock).not.toHaveBeenCalled();
 
       getClientSpy.mockRestore();
     });