feat(hono): Add base for Sentry Hono middleware (Cloudflare)#18787
feat(hono): Add base for Sentry Hono middleware (Cloudflare)#18787
Conversation
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
s1gr1d
force-pushed
the
sig/hono-package-1-cloudflare
branch
from
0124325 to
872d161
Compare
# Conflicts: # dev-packages/cloudflare-integration-tests/package.json
Codecov Results 📊Generated by Codecov Action |
size-limit report 📦
|
| // fixme - check out what removing this integration changes | ||
| // integrations: integrations => integrations.filter(integration => integration.name !== 'Hono'), |
There was a problem hiding this comment.
I only added this for future reference - this is something that I need to keep in mind
s1gr1d
changed the title
![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 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| if (context.error) { | ||
| getClient()?.captureException(context.error); | ||
| } |
There was a problem hiding this comment.
Missing mechanism option in captureException call
Medium Severity
The captureException call on line 41 is missing the required mechanism option. Per the review rules, when calling captureException, the mechanism must include handled (set to true or false) and type (a proper origin like 'auto.middleware.hono'). Other Sentry integrations (e.g., Node.js Hono at line 115-120, Cloudflare Hono at line 73) consistently include this metadata. The missing mechanism may result in incomplete exception metadata in Sentry.
| app.use( | ||
| '*', | ||
| sentry(app, { | ||
| dsn: process.env.SENTRY_DSN, |
There was a problem hiding this comment.
Using process.env instead of Cloudflare environment bindings
Medium Severity
The test file uses process.env.SENTRY_DSN which is not available in Cloudflare Workers runtime. All other tests in this directory correctly use env.SENTRY_DSN from the Cloudflare environment bindings. Since process.env doesn't exist in Workers, the DSN will be undefined, causing the SDK to not send events. The middleware API needs to accept a callback or accessor function to properly retrieve the DSN from the Cloudflare env object per-request.
|
|
||
| if (context.error) { | ||
| getClient()?.captureException(context.error); | ||
| } |
There was a problem hiding this comment.
Duplicate exception capture from multiple error handlers
High Severity
The responseHandler captures context.error via getClient()?.captureException(context.error), but the Cloudflare SDK's honoIntegration (enabled by default) already captures exceptions via the proxied errorHandler. When an error occurs, Hono sets context.error and invokes the error handler, where honoIntegration.handleHonoException captures it. Then responseHandler runs after next() completes and captures the same error again. This results in duplicate exception reports being sent to Sentry.
packages/hono/src/index.ts
Outdated
| @@ -0,0 +1,2 @@ | |||
| // eslint-disable-next-line @typescript-eslint/explicit-function-return-type | |||
| export const sentryNoOp = () => {}; | |||
There was a problem hiding this comment.
l: Wouldn't it be better to just not export anything? It feels odd to get a suggestion here that doesn't do anything
packages/hono/README.md
Outdated
| ```jsonc {tabTitle:JSON} {filename:wrangler.jsonc} | ||
| { | ||
| "compatibility_flags": [ | ||
| "nodejs_als", |
There was a problem hiding this comment.
l: Let's remove nodejs_als entirely and only recommend nodejs_compat
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| if (context.error) { | ||
| getClient()?.captureException(context.error); | ||
| } |
There was a problem hiding this comment.
Bug: The middleware's check for context.error will not capture errors from route handlers, as Hono processes them through its errorHandler before the middleware can inspect the context.
Severity: MEDIUM
Suggested Fix
Remove the error handling logic from the shared middleware (lines 40-42 in middlewareHandlers.ts). Error capturing should be handled exclusively by instrumenting Hono's errorHandler, which is the established and working pattern in the Cloudflare integration.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/hono/src/shared/middlewareHandlers.ts#L40-L42
Potential issue: The error capturing logic within the `responseHandler` at lines 40-42
relies on checking `context.error`. However, when a route handler in Hono throws an
error, Hono's internal mechanism catches it and invokes the registered `errorHandler`.
The error is not populated in `context.error` at the middleware level for this common
case. The Cloudflare integration already correctly instruments the `errorHandler` via a
proxy. This means the middleware's error check will silently fail to capture the most
common type of errors (from route handlers), creating redundant and ineffective code.
Did we get this right? 👍 / 👎 to inform future reviews.
2 participants
Exporting a hono middleware for applications on Cloudflare with
@sentry/hono/cloudflare.This is the base setup for continuing work on this SDK. It adds the export and some integration tests.
Internal tracking issue: https://linear.app/getsentry/issue/FE-655/hono-add-cloudflare-support-1