ref(hono): move shared Hono instrumentation into @sentry/server-utils - #24496
Merged
Merged
Conversation
mydea
added this pull request to stack #24498
September 18, 2026 10:39
Contributor
size-limit report 📦
|
mydea
force-pushed
the
feat/hono-move-to-server-utils
branch
from
September 21, 2026 08:13
57d3514 to
09ebc47
Compare
![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 09ebc47. Configure here.
s1gr1d
approved these changes
Sep 21, 2026
s1gr1d
left a comment
s1gr1d
left a comment
Member
There was a problem hiding this comment.
LGTM, only one comment about the deleted tests.
Comment on lines
+186
to
+197
| test.describe('error inside internal fetch (degraded response)', () => { | ||
| // The manual `sentry()` middleware only instruments the main app's request lifecycle. An error | ||
| // thrown solely inside an internal sub-app `.request()` — whose failed response the outer handler | ||
| // swallows — is therefore NOT captured here, unlike the orchestrion auto-instrumentation in the | ||
| // `hono-4` app, which instruments every dispatched context. We assert only that the outer request | ||
| // stays healthy. | ||
| test('degrades to a 200 when the inner route fails', async ({ baseURL }) => { | ||
| const response = await fetch(`${baseURL}${STOREFRONT}/product/self-watering-plant/degraded`); | ||
| expect(response.status).toBe(200); | ||
| await expect(response.json()).resolves.toEqual({ product: null, degraded: true }); | ||
| }); | ||
| }); |
Member
There was a problem hiding this comment.
Why was this and wrapMiddlewareSpan deleted?
Member
Author
There was a problem hiding this comment.
we'd need to export this from server-utils to be able to test this in the hono package 😢 not 100% sure...
Member
There was a problem hiding this comment.
ohhh 😬 then we should at least test this E2E
Member
Author
There was a problem hiding this comment.
I added some node-integration tests etc. to cover the things that we used to cover before. I think coverage should be decent now!
mydea
marked this pull request as ready for review
September 21, 2026 12:43
mydea
requested review from
chargome and
nicohrubec
and removed request for
a team
September 21, 2026 12:43
JPeer264
approved these changes
Sep 22, 2026
JPeer264
left a comment
JPeer264
left a comment
Member
There was a problem hiding this comment.
Thanks for splitting that up
mydea
force-pushed
the
feat/hono-move-to-server-utils
branch
from
September 29, 2026 07:41
9f3e320 to
dd97746
Compare
mydea
requested review from
andreiborza and
isaacs
and removed request for
a team
September 29, 2026 07:41
mydea
force-pushed
the
feat/hono-move-to-server-utils
branch
from
September 29, 2026 12:15
dd97746 to
153b2e1
Compare
Relocate the runtime-agnostic Hono instrumentation out of `@sentry/hono` into `packages/server-utils/src/integrations/hono/`, with `@sentry/hono` re-exporting it. Server-utils must not depend on `hono`, so the relocated code no longer imports it (the `Hono` class is passed in by the SDK and a vendored `honoTypes` provides the shapes). The `sentry()` middleware now builds its request handler via the shared `createHonoRequestMiddleware`. No behavior change. Also renames the `hono-4` e2e test app to `hono-4-legacy` and moves the corresponding unit tests alongside the relocated code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Align the middleware definitions and assertions in the hono-4-legacy e2e app with the hono-4 app: anonymous function expressions (name inferred from the const binding, stable when bundled), a unique per-throw error suffix, and matching error assertions. Add a named-function middleware case, and a degraded-response test that documents the manual sentry() middleware does not capture errors thrown solely inside an internal sub-app .request() (unlike the auto-instrumentation). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Cloudflare and Deno Hono adapters imported applyHonoPatches, earlyPatchHono and createHonoRequestMiddleware from the main @sentry/server-utils entry, whose barrel also loads node:diagnostics_channel and other Node-only modules — which can fail at import/bundle time on Cloudflare Workers and Deno. Export the runtime-agnostic helpers from `exports.ts` (shared by both entries), sourced directly from their modules rather than the ./integrations/hono barrel, so they are available from @sentry/server-utils/no-diagnostic-channels without pulling in diagnostics-channel. Point the Cloudflare and Deno adapters at that Node-free entry. Also move the applyHonoPatches wrapper out of the barrel into the Node-free applyPatches module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mydea
force-pushed
the
feat/hono-move-to-server-utils
branch
from
September 30, 2026 07:00
153b2e1 to
cd73831
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| } | ||
|
|
||
| if (input instanceof Request) { | ||
| return new URL(input.url).pathname; |
Contributor
There was a problem hiding this comment.
Bug: The extractPathname function can crash on Request objects with malformed URLs. The call to new URL(input.url) lacks a try...catch block, violating the function's "must never throw" contract.
Severity: HIGH
Suggested Fix
Wrap the new URL(input.url).pathname call within a try...catch block, similar to the handling for string inputs. In the catch block, return a fallback pathname, for example by using the stripQueryAndHash(input.url) utility, to ensure the function never throws an exception.
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/server-utils/src/integrations/hono/patchAppRequest.ts#L66
Potential issue: The `extractPathname` function does not handle potential errors when
parsing the URL from a `Request` object. Specifically, the line `new
URL(input.url).pathname` is not wrapped in a `try...catch` block. While most `Request`
objects have valid URLs, runtimes like Cloudflare Workers can create `Request` objects
with malformed URLs, such as those with invalid percent-encoding. In such cases, `new
URL()` will throw an exception, causing an unhandled error that crashes the request
handler. This violates the function's documented contract that it "must never throw" and
creates an inconsistency with the string-based input path, which does include error
handling.
Did we get this right? 👍 / 👎 to inform future reviews.
mydea
added a commit
that referenced
this pull request
Sep 30, 2026
Second of two stacked PRs splitting the Hono instrumentation rework (originally #24371). Stacked on #24496 — review/merge that first; the diff here is against the base PR's branch. Adds `honoIntegration`, the auto-instrumentation that hooks Hono through the orchestrion module transform (`node:diagnostics_channel`) so requests are route-enriched without a manual `sentry()` middleware, plus its manual counterpart `honoMiddleware`. Registers it in `getErrorIntegrations()` and re-exports both from the server runtimes (node, cloudflare, bun, deno, and the serverless / meta-framework packages). Also adds the orchestrion transform config for `hono`, node-integration-tests for the auto-instrumentation, and a new orchestrion-based `hono-4` e2e app (the middleware-based app now lives as `hono-4-legacy`, added in the base PR). `node-mastra` now asserts route-enriched Hono spans in prod, where Hono is external and orchestrion-instrumented. _Root cause / notable fix:_ pulling the Hono module into the Cloudflare barrel reshuffled the worker bundle's module-init order and surfaced a latent TDZ crash for provided-module integrations (Flue): `Cannot access 'flueIntegration' before initialization` at worker startup. The orchestrion snippet injected into each instrumented module passed the integration factory **by reference**, reading the binding the instant that module evaluated — and since `@sentry/*/vite` imports a provided-module integration back into its own instrumented package, that closes an import cycle. The snippet now wraps the factory in an arrow (`() => flueIntegration()`) so the binding is only read when the stored thunk runs at `init()`, breaking the cycle. Also folds in a few follow-ups: `honoMiddleware` is now exported from the remaining runtimes that only had `honoIntegration` (elysia, remix, solidstart, sveltekit), the shared `INTERNAL_REQUEST_ORIGIN` constant moved into `hono/constants.ts`, and the previously-skipped `.basePath()` middleware e2e test is unskipped — the per-request orchestrion hook discovers middleware from the matched-handler list at dispatch, so it now works on the clone `.basePath()` returns. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: isaacs <i@izs.me> Co-authored-by: Jan Peer Stöcklmair <jan.oster94@gmail.com>
47 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First of two stacked PRs splitting the Hono instrumentation rework (originally #24371).
Relocates the runtime-agnostic Hono instrumentation out of
@sentry/honointo@sentry/server-utils(src/integrations/hono/), with@sentry/honore-exporting it. Since@sentry/server-utilsmust not depend onhono, the relocated code no longer imports it: theHonoclass is passed in by the SDK and a vendoredhonoTypesmodule provides the shapes. Thesentry()middleware now builds its request handler via the sharedcreateHonoRequestMiddleware. Pure relocation — no behavior change.Also renames the
hono-4e2e test app tohono-4-legacyand moves the corresponding unit tests alongside the relocated code.The orchestrion-based auto-instrumentation that builds on this lives in the stacked PR #24497.
🤖 Generated with Claude Code