feat(hono)!: Change setup for @sentry/hono/node (init in external file)#20497
feat(hono)!: Change setup for @sentry/hono/node (init in external file)#20497
@sentry/hono/node (init in external file)#20497Conversation
size-limit report 📦
|
| 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 |
There was a problem hiding this comment.
l: maybe just Initialize Sentry?
There was a problem hiding this comment.
Hm, I would rather keep this very explicit
| 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: |
There was a problem hiding this comment.
m: if users provide options here again, would that be silently ignored? if that is the case I think it would be good to be a bit more explicit about that people should not use these together
There was a problem hiding this comment.
Yes, I added a note/warning about that in the JSDoc to sentry but I can call that out here again. I am wondering if I should just delete the options param...there is no need for really having it.
| export function init(options: HonoNodeOptions): Client | undefined { | ||
| const existingClient = getClient(); | ||
| if (existingClient) { | ||
| debug.log('Sentry is already initialized, skipping re-initialization.'); |
There was a problem hiding this comment.
h: should we also log this outside of debug mode? feels like this would be easy to trip over for users
There was a problem hiding this comment.
I think would be great if we have this generally everywhere 🤔
| init(options); | ||
| export const sentry = (app: Hono, options?: HonoNodeOptions): MiddlewareHandler => { | ||
| if (options) { | ||
| options.debug && debug.log('Initialized Sentry Hono middleware (Node)'); |
There was a problem hiding this comment.
l: should this be moved to after the init call? since init might still bail
s1gr1d
changed the title
@sentry/hono/node (init in external file)@sentry/hono/node (init in external file)
JPeer264
left a comment
JPeer264
left a comment
There was a problem hiding this comment.
LGTM and everything what Nico wrote
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ec1df64. Configure here.
| 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.', | ||
| ); |
There was a problem hiding this comment.
Warning for uninitialized Sentry is silently swallowed
High Severity
The debug.warn() call on the "Sentry is not initialized" path will never produce any output. Internally, debug.warn delegates to _maybeLog, which only logs when both DEBUG_BUILD is true and isEnabled() returns true. The logger is enabled via debug.enable(), which is only called inside initAndBind during Sentry.init(). By definition, when this warning path is reached, init() was never called, so the logger is disabled and the warning is silently swallowed. This is the primary safety net for users who forget the new --import setup step after a breaking change, so it's important that it actually displays. Using consoleSandbox(() => console.warn(...)) instead of debug.warn(...) would work regardless of logger state — this is the same pattern used in initAndBind in packages/core/src/sdk.ts for similar pre-init situations.
Reviewed by Cursor Bugbot for commit ec1df64. Configure here.
3 participants
Let's users set up Sentry in an external file. This makes it easier to filter any integrations. The setup is now slightly different than it is for Bun and Cloudflare, but the Node runtime just works differently.
Closes #20460
BREAKING CHANGE:
sentryfrom@sentry/hono/nodedoes not acceptoptionsanymore as those need to be passed in a separateSentry.init()that is called in an external file: