From 0a7a49046d00f0e7150458ef7c86c5f64db70542 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 20 Oct 2026 12:35:31 +0200 Subject: [PATCH 1/5] get conn info from runtime --- .../hono-4-legacy/tests/multi-fetch.test.ts | 7 +- .../hono-4-legacy/tests/tracing.test.ts | 6 +- .../hono-4/tests/tracing.test.ts | 15 +- packages/hono/package.json | 5 - packages/hono/src/bun/middleware.ts | 3 +- packages/hono/src/cloudflare/middleware.ts | 3 +- packages/hono/src/deno/middleware.ts | 3 +- packages/hono/src/node/middleware.ts | 3 +- packages/hono/test/bun/middleware.test.ts | 24 ++- .../hono/test/cloudflare/middleware.test.ts | 17 ++ packages/hono/test/deno/middleware.test.ts | 24 ++- packages/hono/test/node/middleware.test.ts | 24 ++- .../test/shared/middlewareHandlers.test.ts | 157 +++++++++++------- .../test/shared/requestConnInfoAttributes.ts | 28 ++++ .../src/integrations/hono/connInfo.ts | 61 +++++++ .../integrations/hono/createHonoMiddleware.ts | 14 +- .../src/integrations/hono/honoIntegration.ts | 46 +---- .../src/integrations/hono/honoTypes.ts | 9 - .../integrations/hono/middlewareHandlers.ts | 62 ++++--- .../test/integrations/hono/connInfo.test.ts | 124 ++++++++++++++ .../hono/createHonoMiddleware.test.ts | 18 +- 21 files changed, 466 insertions(+), 187 deletions(-) create mode 100644 packages/hono/test/shared/requestConnInfoAttributes.ts create mode 100644 packages/server-utils/src/integrations/hono/connInfo.ts create mode 100644 packages/server-utils/test/integrations/hono/connInfo.test.ts diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts index eecd5b99bca4..7db1a77bdab8 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts @@ -360,8 +360,11 @@ test.describe('multi-fetch: internal .request() calls between sub-apps', () => { }); test('error from failed internal fetch is correlated with the storefront trace', async ({ baseURL }) => { + // Use a param unique to this test: the `no error status` test above fetches `/ghost` too, and its + // identical `Failed to fetch product: ghost` error would otherwise be dropped by the Dedupe + // integration, so this test's `waitForError` would never fire. const errorPromise = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === 'Failed to fetch product: ghost'; + return event.exception?.values?.[0]?.value === 'Failed to fetch product: phantom'; }); const segmentPromise = waitForStreamedSpan( @@ -372,7 +375,7 @@ test.describe('multi-fetch: internal .request() calls between sub-apps', () => { segment.name === `GET ${STOREFRONT}/product-or-throw/:productId`, ); - await fetch(`${baseURL}${STOREFRONT}/product-or-throw/ghost`); + await fetch(`${baseURL}${STOREFRONT}/product-or-throw/phantom`); const [errorEvent, segment] = await Promise.all([errorPromise, segmentPromise]); diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/tracing.test.ts b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/tracing.test.ts index 23ed5c7837fc..476eb6e7ab3a 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/tracing.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/tracing.test.ts @@ -48,8 +48,8 @@ test('attaches HTTP connection info to the server span', async ({ baseURL, page expect(data['client.address']?.value).toEqual(expect.any(String)); expect(data['network.peer.address']?.value).toBe(data['client.address']?.value); - if (RUNTIME !== 'deno') { - // Only exposed in `hono/deno` + if (RUNTIME !== 'node' && RUNTIME !== 'deno') { + // Only set by the Node HTTP server span and read from Deno's `remoteAddr` expect(data['network.transport']?.value).toBeUndefined(); } else { expect(data['network.transport']?.value).toMatch(/tcp/); @@ -99,7 +99,7 @@ test("preserves the baseline server.*, client.* and network.* server span attrib expect(data['network.type']?.value).toMatch(/^ipv[46]$/); expect(data['network.protocol.name']?.value).toBe('http'); expect(data['network.protocol.version']?.value).toBe('1.1'); - expect(data['network.transport']?.value).toBeUndefined(); + expect(data['network.transport']?.value).toBe('tcp'); expect(data['network.local.port']?.value).toBe(data['server.port']?.value); expect(data['network.local.address']?.value).toEqual(expect.any(String)); expect(data['network.peer.address']?.value).toBe(data['client.address']?.value); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts index 9350259d7e5c..cb8c4789f361 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts @@ -6,7 +6,7 @@ const anyString = expect.any(String) as unknown; const anyNumber = expect.any(Number) as unknown; const ipvType = expect.stringMatching(/^ipv[46]$/) as unknown; -// Per-runtime `http.server` connection-info expectations. Each runtime's `getConnInfo` helper exposes +// Per-runtime `http.server` connection-info expectations. Each runtime object that Hono holds exposes // a different set of fields, so the expected attribute values are declared here once (keyed by // runtime) instead of branching inside the tests. `undefined` means the attribute must be absent. // Relational checks (`network.peer.*` mirroring `client.*`) are asserted in the tests, since they @@ -21,7 +21,7 @@ const CONN_INFO: Record; baseline: Rec 'client.address': anyString, 'client.port': anyNumber, 'network.type': ipvType, - 'network.transport': undefined, + 'network.transport': 'tcp', }, baseline: { 'server.address': 'localhost', @@ -30,7 +30,7 @@ const CONN_INFO: Record; baseline: Rec 'network.type': ipvType, 'network.protocol.name': 'http', 'network.protocol.version': '1.1', - 'network.transport': undefined, + 'network.transport': 'tcp', 'network.local.address': anyString, 'network.local.port': anyNumber, }, @@ -45,7 +45,7 @@ const CONN_INFO: Record; baseline: Rec baseline: { 'client.port': anyNumber, 'network.type': ipvType }, }, deno: { - // Only `hono/deno` exposes `network.transport`. + // Only Deno's `remoteAddr` exposes `network.transport`. added: { 'client.address': anyString, 'client.port': anyNumber, 'network.transport': expect.stringMatching(/tcp/) }, baseline: { 'server.address': 'localhost', @@ -55,11 +55,10 @@ const CONN_INFO: Record; baseline: Rec }, }, cloudflare: { - // Cloudflare Workers expose no client address, port, address family, or transport: there is no - // `getConnInfo` on Workers and nothing else populates `client.address`, so all of these are - // absent. Asserting `undefined` here lets us notice if that ever changes. + // Cloudflare Workers expose only the client address (the `cf-connecting-ip` header): no port, + // address family, or transport. added: { - 'client.address': undefined, + 'client.address': anyString, 'client.port': undefined, 'network.type': undefined, 'network.transport': undefined, diff --git a/packages/hono/package.json b/packages/hono/package.json index 9fded7c37b46..fb82583696f8 100644 --- a/packages/hono/package.json +++ b/packages/hono/package.json @@ -79,7 +79,6 @@ }, "peerDependencies": { "@cloudflare/workers-types": "^4.x || ^5.x", - "@hono/node-server": "^1.x || ^2.x", "@sentry/bun": "11.7.0", "@sentry/cloudflare": "11.7.0", "@sentry/deno": "11.7.0", @@ -90,9 +89,6 @@ "@cloudflare/workers-types": { "optional": true }, - "@hono/node-server": { - "optional": true - }, "@sentry/bun": { "optional": true }, @@ -108,7 +104,6 @@ }, "devDependencies": { "@cloudflare/workers-types": "4.20250922.0", - "@hono/node-server": "^2.0.10", "@types/node": "^18.19.1", "wrangler": "4.62.0" }, diff --git a/packages/hono/src/bun/middleware.ts b/packages/hono/src/bun/middleware.ts index 0ca1a4e92def..22182fe0dfac 100644 --- a/packages/hono/src/bun/middleware.ts +++ b/packages/hono/src/bun/middleware.ts @@ -1,7 +1,6 @@ import type { BunOptions } from '@sentry/bun'; import { debug } from '@sentry/core'; import { init } from './sdk'; -import { getConnInfo } from 'hono/bun'; import { applyHonoPatches, createHonoRequestMiddleware, type SentryHonoMiddlewareOptions } from '@sentry/server-utils'; import type { Env, Hono, MiddlewareHandler } from 'hono'; @@ -17,5 +16,5 @@ export const sentry = (app: Hono, options: HonoBunOptions): Mi applyHonoPatches(app); - return createHonoRequestMiddleware({ getConnInfo, shouldHandleError: options.shouldHandleError }); + return createHonoRequestMiddleware({ runtime: 'bun', shouldHandleError: options.shouldHandleError }); }; diff --git a/packages/hono/src/cloudflare/middleware.ts b/packages/hono/src/cloudflare/middleware.ts index ef5349c7d7d6..19a2417a5ad1 100644 --- a/packages/hono/src/cloudflare/middleware.ts +++ b/packages/hono/src/cloudflare/middleware.ts @@ -1,6 +1,5 @@ import { withSentry, type CloudflareOptions } from '@sentry/cloudflare'; import { applySdkMetadata, debug } from '@sentry/core'; -import { getConnInfo } from 'hono/cloudflare-workers'; import { applyHonoPatches, createHonoRequestMiddleware, @@ -46,7 +45,7 @@ export function sentry( applyHonoPatches(app); return createHonoRequestMiddleware({ - getConnInfo, + runtime: 'cloudflare', // Cloudflare accepts middleware options as a function of `env`, so `shouldHandleError` is only // known per request. resolveShouldHandleError: context => diff --git a/packages/hono/src/deno/middleware.ts b/packages/hono/src/deno/middleware.ts index 8a39fc6ecbea..2556c547303e 100644 --- a/packages/hono/src/deno/middleware.ts +++ b/packages/hono/src/deno/middleware.ts @@ -1,7 +1,6 @@ import { debug } from '@sentry/core'; import type { DenoOptions } from '@sentry/deno'; import { init } from './sdk'; -import { getConnInfo } from 'hono/deno'; import { applyHonoPatches, createHonoRequestMiddleware, @@ -21,5 +20,5 @@ export const sentry = (app: Hono, options: HonoDenoOptions): M applyHonoPatches(app); - return createHonoRequestMiddleware({ getConnInfo, shouldHandleError: options.shouldHandleError }); + return createHonoRequestMiddleware({ runtime: 'deno', shouldHandleError: options.shouldHandleError }); }; diff --git a/packages/hono/src/node/middleware.ts b/packages/hono/src/node/middleware.ts index 0d052c6d72ed..cc485923c77c 100644 --- a/packages/hono/src/node/middleware.ts +++ b/packages/hono/src/node/middleware.ts @@ -1,6 +1,5 @@ import { consoleSandbox, debug, getClient } from '@sentry/core'; import type { NodeOptions } from '@sentry/node'; -import { getConnInfo } from '@hono/node-server/conninfo'; import { applyHonoPatches, createHonoRequestMiddleware, type SentryHonoMiddlewareOptions } from '@sentry/server-utils'; import type { Env, Hono, MiddlewareHandler } from 'hono'; @@ -41,5 +40,5 @@ export const sentry = (app: Hono, options?: SentryHonoMiddlewa applyHonoPatches(app); - return createHonoRequestMiddleware({ getConnInfo, shouldHandleError: options?.shouldHandleError }); + return createHonoRequestMiddleware({ runtime: 'node', shouldHandleError: options?.shouldHandleError }); }; diff --git a/packages/hono/test/bun/middleware.test.ts b/packages/hono/test/bun/middleware.test.ts index ca8fe21d1b45..1e126529c172 100644 --- a/packages/hono/test/bun/middleware.test.ts +++ b/packages/hono/test/bun/middleware.test.ts @@ -5,17 +5,12 @@ import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest'; import { sentry, type HonoBunOptions } from '../../src/bun/middleware'; import { init } from '../../src/bun/sdk'; import { LOW_QUALITY_TRANSACTION_PATTERNS } from '../../src/shared/lowQualityTransactionPatterns'; +import { requestConnInfoAttributes } from '../shared/requestConnInfoAttributes'; vi.mock('@sentry/bun', () => ({ init: vi.fn(), })); -// `hono/bun` eagerly imports Bun-only modules (e.g. SSG) that reference the `Bun` global, -// which is not available under Vitest/Node. We only use its `getConnInfo` helper. -vi.mock('hono/bun', () => ({ - getConnInfo: vi.fn(() => ({ remote: {} })), -})); - // eslint-disable-next-line @typescript-eslint/consistent-type-imports const { init: initBunMock } = await vi.importMock('@sentry/bun'); @@ -254,3 +249,20 @@ describe('Hono Bun Middleware', () => { }); }); }); + +describe('Hono Bun Middleware — connection info', () => { + it('reads the client address from `requestIP` of the server', async () => { + const middleware = sentry(new Hono(), { dsn: 'https://public@dsn.ingest.sentry.io/1337' }); + // `Bun.serve` passes the server to `app.fetch` as `env`. + const env = { requestIP: () => ({ address: '::1', port: 54321, family: 'IPv6' }) }; + + const attributes = await requestConnInfoAttributes(middleware, { env }); + + expect(attributes['client.address']).toBe('::1'); + expect(attributes['client.port']).toBe(54321); + expect(attributes['network.peer.address']).toBe('::1'); + expect(attributes['network.peer.port']).toBe(54321); + expect(attributes['network.type']).toBe('ipv6'); + expect(attributes).not.toHaveProperty('network.transport'); + }); +}); diff --git a/packages/hono/test/cloudflare/middleware.test.ts b/packages/hono/test/cloudflare/middleware.test.ts index 7c50b2823dfa..08a7ff1be619 100644 --- a/packages/hono/test/cloudflare/middleware.test.ts +++ b/packages/hono/test/cloudflare/middleware.test.ts @@ -5,6 +5,7 @@ import { Hono } from 'hono'; import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest'; import { sentry, type HonoCloudflareOptions } from '../../src/cloudflare/middleware'; import { LOW_QUALITY_TRANSACTION_PATTERNS } from '../../src/shared/lowQualityTransactionPatterns'; +import { requestConnInfoAttributes } from '../shared/requestConnInfoAttributes'; vi.mock('@sentry/cloudflare', { spy: true }); vi.mock('@sentry/core', async () => { @@ -197,3 +198,19 @@ describe('Hono Cloudflare Middleware', () => { }); }); }); + +describe('Hono Cloudflare Middleware — connection info', () => { + it('reads the `cf-connecting-ip` header', async () => { + const middleware = sentry(new Hono(), { dsn: 'https://public@dsn.ingest.sentry.io/1337' }); + const headers = { 'cf-connecting-ip': '203.0.113.5' }; + + const attributes = await requestConnInfoAttributes(middleware, { headers }); + + expect(attributes['client.address']).toBe('203.0.113.5'); + expect(attributes['network.peer.address']).toBe('203.0.113.5'); + expect(attributes).not.toHaveProperty('client.port'); + expect(attributes).not.toHaveProperty('network.peer.port'); + expect(attributes).not.toHaveProperty('network.type'); + expect(attributes).not.toHaveProperty('network.transport'); + }); +}); diff --git a/packages/hono/test/deno/middleware.test.ts b/packages/hono/test/deno/middleware.test.ts index bf19b51804b4..937af53ba78c 100644 --- a/packages/hono/test/deno/middleware.test.ts +++ b/packages/hono/test/deno/middleware.test.ts @@ -5,17 +5,12 @@ import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest'; import { sentry, type HonoDenoOptions } from '../../src/deno/middleware'; import { init } from '../../src/deno/sdk'; import { LOW_QUALITY_TRANSACTION_PATTERNS } from '../../src/shared/lowQualityTransactionPatterns'; +import { requestConnInfoAttributes } from '../shared/requestConnInfoAttributes'; vi.mock('@sentry/deno', () => ({ init: vi.fn(), })); -// `hono/deno` references the `Deno` global at module-eval time, which is not defined in the -// vitest (Node) environment. -vi.mock('hono/deno', () => ({ - getConnInfo: vi.fn(), -})); - // eslint-disable-next-line @typescript-eslint/consistent-type-imports const { init: initDenoMock } = await vi.importMock('@sentry/deno'); @@ -254,3 +249,20 @@ describe('Hono Deno Middleware', () => { }); }); }); + +describe('Hono Deno Middleware — connection info', () => { + it('reads the `remoteAddr` of the serve handler info', async () => { + const middleware = sentry(new Hono(), { dsn: 'https://public@dsn.ingest.sentry.io/1337' }); + // `Deno.serve` passes the `ServeHandlerInfo` to `app.fetch` as `env`. + const env = { remoteAddr: { hostname: '127.0.0.1', port: 54321, transport: 'tcp' } }; + + const attributes = await requestConnInfoAttributes(middleware, { env }); + + expect(attributes['client.address']).toBe('127.0.0.1'); + expect(attributes['client.port']).toBe(54321); + expect(attributes['network.peer.address']).toBe('127.0.0.1'); + expect(attributes['network.peer.port']).toBe(54321); + expect(attributes['network.transport']).toBe('tcp'); + expect(attributes).not.toHaveProperty('network.type'); + }); +}); diff --git a/packages/hono/test/node/middleware.test.ts b/packages/hono/test/node/middleware.test.ts index 307eefe05500..a3d408cae92d 100644 --- a/packages/hono/test/node/middleware.test.ts +++ b/packages/hono/test/node/middleware.test.ts @@ -3,15 +3,12 @@ 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'; +import { requestConnInfoAttributes } from '../shared/requestConnInfoAttributes'; vi.mock('@sentry/node', () => ({ init: vi.fn(), })); -vi.mock('@hono/node-server/conninfo', () => ({ - getConnInfo: vi.fn(() => ({ remote: {} })), -})); - // eslint-disable-next-line @typescript-eslint/consistent-type-imports const { init: initNodeMock } = await vi.importMock('@sentry/node'); @@ -232,3 +229,22 @@ describe('Hono Node Middleware', () => { }); }); }); + +describe('Hono Node Middleware — connection info', () => { + it('reads the socket of the incoming message', async () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + const middleware = sentry(new Hono()); + consoleWarnSpy.mockRestore(); + // The `env` that `@hono/node-server` passes to `app.fetch`. + const env = { incoming: { socket: { remoteAddress: '203.0.113.5', remotePort: 54321, remoteFamily: 'IPv4' } } }; + + const attributes = await requestConnInfoAttributes(middleware, { env }); + + expect(attributes['client.address']).toBe('203.0.113.5'); + expect(attributes['client.port']).toBe(54321); + expect(attributes['network.peer.address']).toBe('203.0.113.5'); + expect(attributes['network.peer.port']).toBe(54321); + expect(attributes['network.type']).toBe('ipv4'); + expect(attributes).not.toHaveProperty('network.transport'); + }); +}); diff --git a/packages/hono/test/shared/middlewareHandlers.test.ts b/packages/hono/test/shared/middlewareHandlers.test.ts index 9253cf56cbd6..1cc1bf745b11 100644 --- a/packages/hono/test/shared/middlewareHandlers.test.ts +++ b/packages/hono/test/shared/middlewareHandlers.test.ts @@ -1,7 +1,7 @@ import { HTTP_ROUTE, SENTRY_SEGMENT_NAME_SOURCE } from '@sentry/conventions/attributes'; import * as SentryCore from '@sentry/core'; import { createHonoRequestMiddleware } from '@sentry/server-utils'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // These exercise the request/response handlers via the public `createHonoRequestMiddleware`, which // runs `requestHandler` on the way in and `responseHandler` on the way out of `next()`. @@ -11,16 +11,9 @@ const mockSetSDKProcessingMetadata = vi.fn(); const mockSetUser = vi.fn(); const mockGetUser = vi.fn<() => Record>(() => ({})); -let rootSpanAttributes: Record = {}; const mockRootSpan = { - setAttribute: vi.fn((key: string, value: unknown) => { - rootSpanAttributes[key] = value; - }), - setAttributes: vi.fn((attributes: Record) => { - for (const [key, value] of Object.entries(attributes)) { - rootSpanAttributes[key] = value; - } - }), + setAttribute: vi.fn(), + setAttributes: vi.fn(), updateName: vi.fn(), }; @@ -44,10 +37,11 @@ vi.mock('@sentry/core', async () => { const getClientMock = SentryCore.getClient as ReturnType; const captureExceptionMock = SentryCore.captureException as ReturnType; const getActiveSpanMock = SentryCore.getActiveSpan as ReturnType; +const getRootSpanMock = SentryCore.getRootSpan as ReturnType; // `event` throws on access so `hasFetchEvent` reports `false` (the Node/Bun/Deno path), which makes // the request handler read request data from `req.raw` rather than a Cloudflare fetch event. -function createMockContext(status: number, error?: Error): unknown { +function createMockContext(status: number, error?: Error): object { return { req: { method: 'GET', @@ -68,7 +62,7 @@ const noop = async (): Promise => {}; // oxlint-disable-next-line typescript/no-explicit-any async function runMiddleware(context: unknown, options: Record = {}): Promise { - const middleware = createHonoRequestMiddleware(options); + const middleware = createHonoRequestMiddleware({ runtime: 'node', ...options }); // oxlint-disable-next-line typescript/no-explicit-any await middleware(context as any, noop); } @@ -205,17 +199,22 @@ describe('responseHandler (via createHonoRequestMiddleware)', () => { }); describe('requestHandler — connection info (via createHonoRequestMiddleware)', () => { - const activeSpan = { updateName: vi.fn(), setAttribute: vi.fn() }; + let rootSpan: SentryCore.SentrySpan; beforeEach(() => { vi.clearAllMocks(); - rootSpanAttributes = {}; mockGetUser.mockReturnValue({}); - getActiveSpanMock.mockReturnValue(activeSpan); + rootSpan = new SentryCore.SentrySpan({ name: 'GET /test' }); + getActiveSpanMock.mockReturnValue(rootSpan); + getRootSpanMock.mockReturnValue(rootSpan); }); - function getConnInfoStub(remote: Record): () => { remote: Record } { - return vi.fn(() => ({ remote })); + afterEach(() => { + getRootSpanMock.mockReturnValue(mockRootSpan); + }); + + function getRootSpanAttributes(): Record { + return SentryCore.spanToJSON(rootSpan).attributes ?? {}; } function mockUserInfo(userInfo: boolean): void { @@ -224,37 +223,39 @@ describe('requestHandler — connection info (via createHonoRequestMiddleware)', }); } - it('sets non-PII attributes (port, transport, type) regardless of userInfo', async () => { + // The `c.env` that `@hono/node-server` passes to `app.fetch`. + function createNodeContext(socket: Record): object { + return Object.assign(createMockContext(200), { env: { incoming: { socket } } }); + } + + it('sets the peer port and address type regardless of userInfo', async () => { mockUserInfo(false); - const getConnInfo = getConnInfoStub({ port: 54321, transport: 'tcp', addressType: 'IPv4' }); - await runMiddleware(createMockContext(200), { getConnInfo }); + await runMiddleware(createNodeContext({ remoteAddress: '203.0.113.5', remotePort: 54321, remoteFamily: 'IPv4' })); - expect(rootSpanAttributes['client.port']).toBe(54321); - expect(rootSpanAttributes['network.peer.port']).toBe(54321); - expect(rootSpanAttributes['network.transport']).toBe('tcp'); - expect(rootSpanAttributes['network.type']).toBe('ipv4'); - expect(rootSpanAttributes['client.address']).toBeUndefined(); + const attributes = getRootSpanAttributes(); + expect(attributes['network.peer.port']).toBe(54321); + expect(attributes['network.type']).toBe('ipv4'); }); it('sets IP-bearing attributes and user.ip_address when userInfo is true', async () => { mockUserInfo(true); - const getConnInfo = getConnInfoStub({ address: '203.0.113.5', port: 443, addressType: 'IPv6' }); - await runMiddleware(createMockContext(200), { getConnInfo }); + await runMiddleware(createNodeContext({ remoteAddress: '203.0.113.5', remotePort: 443, remoteFamily: 'IPv6' })); - expect(rootSpanAttributes['client.address']).toBe('203.0.113.5'); - expect(rootSpanAttributes['network.peer.address']).toBe('203.0.113.5'); - expect(rootSpanAttributes['network.type']).toBe('ipv6'); + const attributes = getRootSpanAttributes(); + expect(attributes['client.address']).toBe('203.0.113.5'); + expect(attributes['client.port']).toBe(443); + expect(attributes['network.peer.address']).toBe('203.0.113.5'); + expect(attributes['network.type']).toBe('ipv6'); expect(mockSetUser).toHaveBeenCalledWith({ ip_address: '203.0.113.5' }); }); it('merges ip_address into the existing user without overwriting other fields', async () => { mockUserInfo(true); mockGetUser.mockReturnValue({ id: 'user-123', email: 'jane@example.com' }); - const getConnInfo = getConnInfoStub({ address: '203.0.113.5', port: 443 }); - await runMiddleware(createMockContext(200), { getConnInfo }); + await runMiddleware(createNodeContext({ remoteAddress: '203.0.113.5', remotePort: 443 })); expect(mockSetUser).toHaveBeenCalledWith({ id: 'user-123', @@ -265,60 +266,102 @@ describe('requestHandler — connection info (via createHonoRequestMiddleware)', it('omits IP-bearing attributes when userInfo is false', async () => { mockUserInfo(false); - const getConnInfo = getConnInfoStub({ address: '203.0.113.5', port: 8080 }); - await runMiddleware(createMockContext(200), { getConnInfo }); + await runMiddleware(createNodeContext({ remoteAddress: '203.0.113.5', remotePort: 8080 })); - expect(rootSpanAttributes['client.address']).toBeUndefined(); - expect(rootSpanAttributes['network.peer.address']).toBeUndefined(); + const attributes = getRootSpanAttributes(); + expect(attributes).not.toHaveProperty('client.address'); + expect(attributes).not.toHaveProperty('client.port'); + expect(attributes).not.toHaveProperty('network.peer.address'); expect(mockSetUser).not.toHaveBeenCalled(); // Non-PII data is still recorded. - expect(rootSpanAttributes['client.port']).toBe(8080); + expect(attributes['network.peer.port']).toBe(8080); }); - it('sets no connection attributes when remote info is empty', async () => { + it('does not write the attributes that the runtime does not give', async () => { mockUserInfo(true); - const getConnInfo = getConnInfoStub({}); + rootSpan.setAttribute('network.transport', 'tcp'); - await runMiddleware(createMockContext(200), { getConnInfo }); + await runMiddleware(createNodeContext({ remotePort: 54321 })); - expect(rootSpanAttributes['client.port']).toBeUndefined(); - expect(rootSpanAttributes['network.peer.port']).toBeUndefined(); - expect(rootSpanAttributes['network.transport']).toBeUndefined(); - expect(rootSpanAttributes['network.type']).toBeUndefined(); - expect(rootSpanAttributes['client.address']).toBeUndefined(); + const attributes = getRootSpanAttributes(); + expect(attributes['network.transport']).toBe('tcp'); + expect(attributes).not.toHaveProperty('client.address'); + expect(attributes).not.toHaveProperty('network.type'); expect(mockSetUser).not.toHaveBeenCalled(); }); - it('does not throw or set attributes when getConnInfo throws', async () => { + it('does not throw or set attributes when a runtime object throws', async () => { mockUserInfo(true); - const getConnInfo = vi.fn(() => { - throw new Error('socket unavailable'); + const incoming = Object.defineProperty({}, 'socket', { + get() { + throw new Error('socket unavailable'); + }, }); + const context = Object.assign(createMockContext(200), { env: { incoming } }); - await expect(runMiddleware(createMockContext(200), { getConnInfo })).resolves.toBeUndefined(); - expect(rootSpanAttributes['client.port']).toBeUndefined(); - expect(rootSpanAttributes['client.address']).toBeUndefined(); + await expect(runMiddleware(context)).resolves.toBeUndefined(); + expect(getRootSpanAttributes()).not.toHaveProperty('client.port'); expect(mockSetUser).not.toHaveBeenCalled(); }); it('does not set connection attributes when there is no active span', async () => { mockUserInfo(true); getActiveSpanMock.mockReturnValue(null); - const getConnInfo = getConnInfoStub({ address: '203.0.113.5', port: 443 }); - await runMiddleware(createMockContext(200), { getConnInfo }); + await runMiddleware(createNodeContext({ remoteAddress: '203.0.113.5', remotePort: 443 })); - expect(getConnInfo).not.toHaveBeenCalled(); - expect(rootSpanAttributes).toEqual({}); + const attributes = getRootSpanAttributes(); + expect(attributes).not.toHaveProperty('client.address'); + expect(attributes).not.toHaveProperty('network.peer.port'); + expect(mockSetUser).not.toHaveBeenCalled(); }); - it('is a no-op when getConnInfo is not provided', async () => { + it('sets no connection attributes when the runtime objects are missing', async () => { mockUserInfo(true); await runMiddleware(createMockContext(200)); - expect(rootSpanAttributes['client.port']).toBeUndefined(); + const attributes = getRootSpanAttributes(); + expect(attributes).not.toHaveProperty('client.port'); + expect(attributes).not.toHaveProperty('client.address'); expect(mockSetUser).not.toHaveBeenCalled(); }); + + it('sets the transport that the runtime gives', async () => { + mockUserInfo(false); + const context = Object.assign(createMockContext(200), { + env: { remoteAddr: { hostname: '203.0.113.5', port: 54321, transport: 'tcp' } }, + }); + + await runMiddleware(context, { runtime: 'deno' }); + + expect(getRootSpanAttributes()['network.transport']).toBe('tcp'); + }); + + it('keeps attributes that are already on the root span', async () => { + mockUserInfo(true); + rootSpan.setAttributes({ 'client.address': '198.51.100.7', 'client.port': 1234 }); + + await runMiddleware(createNodeContext({ remoteAddress: '10.0.0.1', remotePort: 54321 })); + + const attributes = getRootSpanAttributes(); + expect(attributes['client.address']).toBe('198.51.100.7'); + expect(attributes['client.port']).toBe(1234); + expect(attributes['network.peer.address']).toBe('10.0.0.1'); + expect(attributes['network.peer.port']).toBe(54321); + }); + + it('does not pair the socket port with a client.address that is already on the root span', async () => { + mockUserInfo(true); + // The runtime SDK read `client.address` from a forwarding header and knows no client port. + rootSpan.setAttribute('client.address', '198.51.100.7'); + + await runMiddleware(createNodeContext({ remoteAddress: '10.0.0.1', remotePort: 54321 })); + + const attributes = getRootSpanAttributes(); + expect(attributes['client.address']).toBe('198.51.100.7'); + expect(attributes).not.toHaveProperty('client.port'); + expect(attributes['network.peer.port']).toBe(54321); + }); }); diff --git a/packages/hono/test/shared/requestConnInfoAttributes.ts b/packages/hono/test/shared/requestConnInfoAttributes.ts new file mode 100644 index 000000000000..b54fe80e6587 --- /dev/null +++ b/packages/hono/test/shared/requestConnInfoAttributes.ts @@ -0,0 +1,28 @@ +import * as SentryCore from '@sentry/core'; +import { Hono, type MiddlewareHandler } from 'hono'; +import { vi } from 'vitest'; + +/** + * Sends one request through `middleware` inside a root span, with `userInfo` collection on, and + * gives the root span attributes. `env` is what the runtime adapter passes to `app.fetch`. + */ +export async function requestConnInfoAttributes( + middleware: MiddlewareHandler, + { env, headers }: { env?: unknown; headers?: Record }, +): Promise> { + const app = new Hono(); + app.use(middleware); + app.get('/', c => c.text('ok')); + + const fakeClient = { getDataCollectionOptions: () => ({ userInfo: true }) }; + const getClientSpy = vi.spyOn(SentryCore, 'getClient').mockReturnValue(fakeClient as unknown as SentryCore.Client); + const rootSpan = new SentryCore.SentrySpan({ name: 'GET /' }); + + try { + await SentryCore.withActiveSpan(rootSpan, () => app.request('/', { headers }, env)); + } finally { + getClientSpy.mockRestore(); + } + + return SentryCore.spanToJSON(rootSpan).attributes ?? {}; +} diff --git a/packages/server-utils/src/integrations/hono/connInfo.ts b/packages/server-utils/src/integrations/hono/connInfo.ts new file mode 100644 index 000000000000..ac72f2154363 --- /dev/null +++ b/packages/server-utils/src/integrations/hono/connInfo.ts @@ -0,0 +1,61 @@ +import type { Context } from './honoTypes'; + +export type HonoRuntime = 'node' | 'bun' | 'deno' | 'cloudflare'; + +/** The client side of the request's connection. */ +export interface ConnInfo { + address?: string; + port?: number; + transport?: string; + addressType?: string; +} + +// Structural shapes of what each runtime adapter passes to `app.fetch` as `c.env`. They copy the reads of +// Hono's own `getConnInfo` helpers, which moved out of `hono` in Hono 5. +// The readers cast `c.env` to these shapes without validation: the caller selects the runtime, the runtime +// defines the shape, and `readConnInfo` catches a read that throws. +type NodeBindings = { + incoming?: { socket?: { remoteAddress?: string; remotePort?: number; remoteFamily?: string } }; +}; +type BunServer = { + requestIP?: (request: Request) => { address?: string; port?: number; family?: string } | null; +}; +type DenoServeHandlerInfo = { remoteAddr?: { hostname?: string; port?: number; transport?: string } }; + +const READERS: Record ConnInfo | undefined> = { + node: context => { + const env = context.env as (NodeBindings & { server?: NodeBindings }) | undefined; + const socket = (env?.server ?? env)?.incoming?.socket; + return socket + ? { address: socket.remoteAddress, port: socket.remotePort, addressType: socket.remoteFamily } + : undefined; + }, + bun: context => { + const env = context.env as (BunServer & { server?: BunServer }) | undefined; + const info = (env?.server ?? env)?.requestIP?.(context.req.raw); + return info ? { address: info.address, port: info.port, addressType: info.family } : undefined; + }, + deno: context => { + const remoteAddr = (context.env as DenoServeHandlerInfo | undefined)?.remoteAddr; + return remoteAddr + ? { address: remoteAddr.hostname, port: remoteAddr.port, transport: remoteAddr.transport } + : undefined; + }, + // On Cloudflare `c.env` holds the user's bindings, so only the request is read. + cloudflare: context => { + const address = context.req.raw.headers.get('cf-connecting-ip'); + return address ? { address } : undefined; + }, +}; + +/** + * Reads the connection info from the runtime objects that Hono holds for the request. + * Gives `undefined` when the runtime object is missing or a read throws. + */ +export function readConnInfo(context: Context, runtime: HonoRuntime): ConnInfo | undefined { + try { + return READERS[runtime](context); + } catch { + return undefined; + } +} diff --git a/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts b/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts index 74f0a96ca809..24ffcfdb00aa 100644 --- a/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts +++ b/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts @@ -1,5 +1,6 @@ import { addNonEnumerableProperty, getDefaultIsolationScope, getIsolationScope } from '@sentry/core'; -import type { Context, GetConnInfo, MiddlewareHandler } from './honoTypes'; +import type { HonoRuntime } from './connInfo'; +import type { Context, MiddlewareHandler } from './honoTypes'; import { captureContextError, requestHandler, responseHandler } from './middlewareHandlers'; import type { SentryHonoMiddlewareOptions } from './types'; @@ -16,11 +17,8 @@ const HONO_REQUEST_HANDLED = Symbol.for('sentry.hono.requestHandled'); const HONO_SHOULD_HANDLE_ERROR = Symbol.for('sentry.hono.shouldHandleError'); export interface CreateHonoRequestMiddlewareOptions { - /** - * Runtime-specific `getConnInfo` helper (e.g. `@hono/node-server/conninfo`, `hono/bun`). - * Optional — connection-info attributes are simply skipped when it is not provided. - */ - getConnInfo?: GetConnInfo; + /** The runtime that serves the app. It selects where the connection info is read from. */ + runtime: HonoRuntime; /** Static `shouldHandleError` callback (Node/Bun/Deno). */ shouldHandleError?: SentryHonoMiddlewareOptions['shouldHandleError']; @@ -65,7 +63,7 @@ function getRequestScope(context: Context): Record { * `Context` constructor hook): the deduplicated middleware records its callback on the request scope, and the * middleware that actually runs `responseHandler` uses it in preference to its own default. */ -export function createHonoRequestMiddleware(options: CreateHonoRequestMiddlewareOptions = {}): MiddlewareHandler { +export function createHonoRequestMiddleware(options: CreateHonoRequestMiddlewareOptions): MiddlewareHandler { const middleware: MiddlewareHandler = async (context, next) => { const scope = getRequestScope(context); @@ -92,7 +90,7 @@ export function createHonoRequestMiddleware(options: CreateHonoRequestMiddleware addNonEnumerableProperty(scope, HONO_REQUEST_HANDLED, true); try { - requestHandler(context, options.getConnInfo); + requestHandler(context, options.runtime); await next(); // Handler runs in between Request above ⤴ and Response below ⤵ diff --git a/packages/server-utils/src/integrations/hono/honoIntegration.ts b/packages/server-utils/src/integrations/hono/honoIntegration.ts index c2b504db2dab..b66fe6b50de4 100644 --- a/packages/server-utils/src/integrations/hono/honoIntegration.ts +++ b/packages/server-utils/src/integrations/hono/honoIntegration.ts @@ -1,11 +1,10 @@ import * as diagnosticsChannel from '../../utils/diagnosticsChannel'; -import { createRequire } from 'node:module'; import { SENTRY_OP, SENTRY_ORIGIN } from '@sentry/conventions/attributes'; import { HTTP_SERVER } from '@sentry/conventions/op'; import type { IntegrationFn } from '@sentry/core'; -import { debug, defineIntegration, startInactiveSpan } from '@sentry/core'; -import { DEBUG_BUILD } from '../../debug-build'; -import type { Env, GetConnInfo, Hono, MiddlewareHandler } from './honoTypes'; +import { defineIntegration, startInactiveSpan } from '@sentry/core'; +import type { HonoRuntime } from './connInfo'; +import type { Env, Hono, MiddlewareHandler } from './honoTypes'; import type { OrchestrionChannelContext } from '../../orchestrion/types'; import { CHANNELS } from '../../orchestrion/channels'; import { honoModuleNames } from '../../orchestrion/config/hono'; @@ -30,40 +29,7 @@ const isBun = typeof globalAny.Bun !== 'undefined'; const isDeno = typeof globalAny.Deno !== 'undefined'; const isCloudflare = globalAny.navigator?.userAgent === 'Cloudflare-Workers'; -let connInfoResolved = false; -let cachedGetConnInfo: GetConnInfo | undefined; - -/** - * Resolves the runtime's `getConnInfo` helper once, best-effort. - * - * Cloudflare Workers can't `require()` out of a bundle at runtime, so there conn-info is left to the - * platform's `requestDataIntegration`. On Node/Bun/Deno a missing optional peer dependency degrades - * to `undefined`, which just skips the connection-info attributes. - */ -function resolveGetConnInfo(): GetConnInfo | undefined { - if (connInfoResolved) { - return cachedGetConnInfo; - } - connInfoResolved = true; - - const specifier = isBun ? 'hono/bun' : isDeno ? 'hono/deno' : isCloudflare ? undefined : '@hono/node-server/conninfo'; - - if (!specifier) { - return undefined; - } - - try { - // `createRequire` resolves relative to its argument's directory, so a never-loaded dummy path - // roots resolution at the app directory, where the runtime helper is installed. - const appRequire = createRequire(`${process.cwd()}/noop.js`); - cachedGetConnInfo = (appRequire(specifier) as { getConnInfo?: GetConnInfo }).getConnInfo; - } catch { - DEBUG_BUILD && debug.log(`[instrumentation:hono] could not resolve \`getConnInfo\` from "${specifier}"`); - cachedGetConnInfo = undefined; - } - - return cachedGetConnInfo; -} +const runtime: HonoRuntime = isBun ? 'bun' : isDeno ? 'deno' : isCloudflare ? 'cloudflare' : 'node'; /** * Manually instruments a Hono app and returns the Sentry request/response middleware to register @@ -78,7 +44,7 @@ export function honoMiddleware(app: Hono, options: HonoIntegra applyPatches(app); return createHonoRequestMiddleware({ - getConnInfo: resolveGetConnInfo(), + runtime, shouldHandleError: options.shouldHandleError, }); } @@ -195,7 +161,7 @@ function instrumentHono(options: HonoIntegrationOptions): void { // The request/response middleware is stateless (per-request state lives on the request scope) and // `options` is fixed, so build it once and reuse it across every dispatched Context. const requestMiddleware = createHonoRequestMiddleware({ - getConnInfo: resolveGetConnInfo(), + runtime, shouldHandleError: options.shouldHandleError, }); diff --git a/packages/server-utils/src/integrations/hono/honoTypes.ts b/packages/server-utils/src/integrations/hono/honoTypes.ts index a4dc01e14152..eadbe688e42a 100644 --- a/packages/server-utils/src/integrations/hono/honoTypes.ts +++ b/packages/server-utils/src/integrations/hono/honoTypes.ts @@ -60,12 +60,3 @@ export interface Hono { routes: HonoRoute[]; [key: string]: any; } - -export interface ConnInfoRemote { - address?: string; - port?: number; - transport?: string; - addressType?: string; -} - -export type GetConnInfo = (context: any) => { remote?: ConnInfoRemote }; diff --git a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts index d72d357b4c86..53909ffaebe5 100644 --- a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts +++ b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts @@ -8,9 +8,12 @@ import { INTERNAL_setSegmentNameSourceIfSegment, updateSpanName, type Scope, + type SpanAttributes, + spanToJSON, winterCGRequestToRequestData, } from '@sentry/core'; -import type { Context, GetConnInfo } from './honoTypes'; +import type { Context } from './honoTypes'; +import { type HonoRuntime, readConnInfo } from './connInfo'; import { hasFetchEvent } from './hono-context'; import { defaultShouldHandleError } from './defaultShouldHandleError'; import { resolveRouteName } from './resolveRouteName'; @@ -20,7 +23,7 @@ import { HTTP_ROUTE } from '@sentry/conventions/attributes'; /** * Request handler for Hono framework */ -export function requestHandler(context: Context, getConnInfo?: GetConnInfo): void { +export function requestHandler(context: Context, runtime: HonoRuntime): void { const isolationScope = getCurrentIsolationScope(); // Set a provisional route name as early as possible so events captured during the request already carry the route. @@ -31,9 +34,7 @@ export function requestHandler(context: Context, getConnInfo?: GetConnInfo): voi normalizedRequest: winterCGRequestToRequestData(hasFetchEvent(context) ? context.event.request : context.req.raw), }); - if (getConnInfo) { - setConnInfoAttributes(context, getConnInfo, isolationScope); - } + setConnInfoAttributes(context, runtime, isolationScope); } function getCurrentIsolationScope(): Scope { @@ -44,42 +45,59 @@ function getCurrentIsolationScope(): Scope { } /** - * Adds HTTP connection info (client IP, port, transport, address type) from Hono's `getConnInfo` - * helper to the root (server) span and the isolation scope. + * Adds HTTP connection info (client IP, port, transport, address type) from the runtime to the root + * (server) span and the isolation scope. + * + * Attributes that are already on the root span are kept: the runtime SDK sets them first and reads + * forwarding headers, while the Node, Bun and Deno runtime objects only know the socket peer (a proxy, + * if there is one). */ -function setConnInfoAttributes(context: Context, getConnInfo: GetConnInfo, isolationScope: Scope): void { +function setConnInfoAttributes(context: Context, runtime: HonoRuntime, isolationScope: Scope): void { const activeSpan = getActiveSpan(); if (!activeSpan) { return; } - let remote: ReturnType['remote'] | undefined; - try { - remote = getConnInfo(context).remote; - } catch { - // The helper can throw when the underlying socket/env is unavailable (e.g. unsupported runtime). + const connInfo = readConnInfo(context, runtime); + if (!connInfo) { return; } - const { address, port, transport, addressType } = remote || {}; + const { address, port, transport, addressType } = connInfo; // Only collect client IP if `userInfo` is enabled (this is primarily for setting data with `.setUser`, but in this case we cannot check for `dataCollection.headers` or similar) const ipAddress = address && getClient()?.getDataCollectionOptions().userInfo ? address : undefined; - getRootSpan(activeSpan).setAttributes({ - 'client.port': port, - 'network.peer.port': port, - 'network.transport': transport, - 'network.type': addressType?.toLowerCase(), - 'client.address': ipAddress, - 'network.peer.address': ipAddress, - }); + const rootSpan = getRootSpan(activeSpan); + const existingAttributes = spanToJSON(rootSpan).attributes ?? {}; + // A port only identifies the client together with its address: behind a proxy, the socket port is the proxy's. + const writesClientAddress = ipAddress !== undefined && existingAttributes['client.address'] === undefined; + rootSpan.setAttributes( + onlyNewAttributes( + { + 'client.port': writesClientAddress ? port : undefined, + 'network.peer.port': port, + 'network.transport': transport, + 'network.type': addressType?.toLowerCase(), + 'client.address': ipAddress, + 'network.peer.address': ipAddress, + }, + existingAttributes, + ), + ); if (ipAddress) { isolationScope.setUser({ ...isolationScope.getUser(), ip_address: ipAddress }); } } +/** Keeps the defined attributes that are not on the span yet. */ +function onlyNewAttributes(attributes: SpanAttributes, existingAttributes: Record): SpanAttributes { + return Object.fromEntries( + Object.entries(attributes).filter(([key, value]) => value !== undefined && existingAttributes[key] === undefined), + ); +} + /** * Response handler for Hono framework */ diff --git a/packages/server-utils/test/integrations/hono/connInfo.test.ts b/packages/server-utils/test/integrations/hono/connInfo.test.ts new file mode 100644 index 000000000000..5e17e10e0f78 --- /dev/null +++ b/packages/server-utils/test/integrations/hono/connInfo.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, it, vi } from 'vitest'; +import { readConnInfo } from '../../../src/integrations/hono/connInfo'; +import type { Context } from '../../../src/integrations/hono/honoTypes'; + +function fakeContext({ env, headers }: { env?: unknown; headers?: Record } = {}): Context { + return { + env, + req: { raw: new Request('http://localhost/test', { headers }) }, + } as unknown as Context; +} + +const throwingGetter = (key: string): object => + Object.defineProperty({}, key, { + get() { + throw new Error('unavailable'); + }, + }); + +describe('readConnInfo', () => { + describe('node', () => { + const socket = { remoteAddress: '203.0.113.5', remotePort: 54321, remoteFamily: 'IPv4' }; + const expected = { address: '203.0.113.5', port: 54321, addressType: 'IPv4' }; + + it('reads the socket of `env.incoming`', () => { + expect(readConnInfo(fakeContext({ env: { incoming: { socket } } }), 'node')).toEqual(expected); + }); + + it('reads the socket of `env.server.incoming`', () => { + expect(readConnInfo(fakeContext({ env: { server: { incoming: { socket } } } }), 'node')).toEqual(expected); + }); + + it.each([ + { case: 'without `env`', env: undefined }, + { case: 'without an incoming message', env: {} }, + ])('returns undefined $case', ({ env }) => { + expect(readConnInfo(fakeContext({ env }), 'node')).toBeUndefined(); + }); + + it('returns undefined when the socket getter throws', () => { + expect(readConnInfo(fakeContext({ env: { incoming: throwingGetter('socket') } }), 'node')).toBeUndefined(); + }); + }); + + describe('bun', () => { + const requestIP = (): object => ({ address: '::1', port: 54321, family: 'IPv6' }); + const expected = { address: '::1', port: 54321, addressType: 'IPv6' }; + + it('calls `requestIP` of the server in `env`', () => { + expect(readConnInfo(fakeContext({ env: { requestIP } }), 'bun')).toEqual(expected); + }); + + it('calls `requestIP` of `env.server`', () => { + expect(readConnInfo(fakeContext({ env: { server: { requestIP } } }), 'bun')).toEqual(expected); + }); + + it('passes the raw request to `requestIP`', () => { + const spy = vi.fn(requestIP); + const context = fakeContext({ env: { requestIP: spy } }); + + readConnInfo(context, 'bun'); + + expect(spy).toHaveBeenCalledWith(context.req.raw); + }); + + it.each([ + { case: 'without a server', env: undefined }, + { case: 'without `requestIP`', env: {} }, + { case: 'when `requestIP` gives null', env: { requestIP: () => null } }, + ])('returns undefined $case', ({ env }) => { + expect(readConnInfo(fakeContext({ env }), 'bun')).toBeUndefined(); + }); + + it('returns undefined when `requestIP` throws', () => { + const env = { + requestIP: () => { + throw new Error('closed'); + }, + }; + + expect(readConnInfo(fakeContext({ env }), 'bun')).toBeUndefined(); + }); + }); + + describe('deno', () => { + it('reads `env.remoteAddr`', () => { + const env = { remoteAddr: { hostname: '127.0.0.1', port: 54321, transport: 'tcp' } }; + + expect(readConnInfo(fakeContext({ env }), 'deno')).toEqual({ + address: '127.0.0.1', + port: 54321, + transport: 'tcp', + }); + }); + + it.each([ + { case: 'without `env`', env: undefined }, + { case: 'without `remoteAddr`', env: {} }, + ])('returns undefined $case', ({ env }) => { + expect(readConnInfo(fakeContext({ env }), 'deno')).toBeUndefined(); + }); + + it('returns undefined when the `remoteAddr` getter throws', () => { + expect(readConnInfo(fakeContext({ env: throwingGetter('remoteAddr') }), 'deno')).toBeUndefined(); + }); + }); + + describe('cloudflare', () => { + it('reads the `cf-connecting-ip` header', () => { + const context = fakeContext({ headers: { 'cf-connecting-ip': '203.0.113.5' } }); + + expect(readConnInfo(context, 'cloudflare')).toEqual({ address: '203.0.113.5' }); + }); + + it('ignores the bindings in `env`', () => { + const env = { incoming: { socket: { remoteAddress: '10.0.0.1' } }, remoteAddr: { hostname: '10.0.0.1' } }; + + expect(readConnInfo(fakeContext({ env }), 'cloudflare')).toBeUndefined(); + }); + + it('returns undefined without the header', () => { + expect(readConnInfo(fakeContext(), 'cloudflare')).toBeUndefined(); + }); + }); +}); diff --git a/packages/server-utils/test/integrations/hono/createHonoMiddleware.test.ts b/packages/server-utils/test/integrations/hono/createHonoMiddleware.test.ts index b0105ead6b02..79cc99806d7e 100644 --- a/packages/server-utils/test/integrations/hono/createHonoMiddleware.test.ts +++ b/packages/server-utils/test/integrations/hono/createHonoMiddleware.test.ts @@ -28,8 +28,8 @@ describe('createHonoRequestMiddleware — duplicate registration handling', () = }); it('runs request/response handling exactly once when two Sentry middlewares wrap the same request', async () => { - const outer = createHonoRequestMiddleware(); - const inner = createHonoRequestMiddleware(); + const outer = createHonoRequestMiddleware({ runtime: 'node' }); + const inner = createHonoRequestMiddleware({ runtime: 'node' }); const context = fakeContext(); // Onion order: outer → inner → handler → inner → outer. @@ -45,8 +45,8 @@ describe('createHonoRequestMiddleware — duplicate registration handling', () = const context = fakeContext(); const handler = vi.fn(); - await createHonoRequestMiddleware()(context, async () => { - await createHonoRequestMiddleware()(context, handler); + await createHonoRequestMiddleware({ runtime: 'node' })(context, async () => { + await createHonoRequestMiddleware({ runtime: 'node' })(context, handler); }); expect(handler).toHaveBeenCalledTimes(1); @@ -56,8 +56,8 @@ describe('createHonoRequestMiddleware — duplicate registration handling', () = it('handles sequential requests sharing one isolation scope independently', async () => { const userShouldHandleError = (): boolean => true; - const auto = createHonoRequestMiddleware(); - const manual = createHonoRequestMiddleware({ shouldHandleError: userShouldHandleError }); + const auto = createHonoRequestMiddleware({ runtime: 'node' }); + const manual = createHonoRequestMiddleware({ runtime: 'node', shouldHandleError: userShouldHandleError }); setAsyncLocalStorageAsyncContextStrategy(); onTestFinished(() => setAsyncContextStrategy(undefined)); @@ -77,7 +77,7 @@ describe('createHonoRequestMiddleware — duplicate registration handling', () = }); it('handles independent requests independently (marker is per-context)', async () => { - const middleware = createHonoRequestMiddleware(); + const middleware = createHonoRequestMiddleware({ runtime: 'node' }); await middleware(fakeContext(), async () => {}); await middleware(fakeContext(), async () => {}); @@ -89,9 +89,9 @@ describe('createHonoRequestMiddleware — duplicate registration handling', () = it("uses a deduplicated middleware's shouldHandleError over the outer (auto) default", async () => { const userShouldHandleError = (): boolean => true; // Outer middleware mirrors the auto-instrumentation: registered first, no shouldHandleError. - const auto = createHonoRequestMiddleware(); + const auto = createHonoRequestMiddleware({ runtime: 'node' }); // Inner middleware mirrors a manual `sentry({ shouldHandleError })`: deduplicated behind `auto`. - const manual = createHonoRequestMiddleware({ shouldHandleError: userShouldHandleError }); + const manual = createHonoRequestMiddleware({ runtime: 'node', shouldHandleError: userShouldHandleError }); const context = fakeContext(); await auto(context, async () => { From 5222ad53c8a6ab8f71c9861f2f6d24fe34b8efa3 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 20 Oct 2026 12:36:29 +0200 Subject: [PATCH 2/5] bundled app test --- .../suites/bundled-app/index.ts | 6 ++++ .../suites/bundled-app/test.ts | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 dev-packages/bun-integration-tests/suites/bundled-app/index.ts create mode 100644 dev-packages/bun-integration-tests/suites/bundled-app/test.ts diff --git a/dev-packages/bun-integration-tests/suites/bundled-app/index.ts b/dev-packages/bun-integration-tests/suites/bundled-app/index.ts new file mode 100644 index 000000000000..fdf313f5a007 --- /dev/null +++ b/dev-packages/bun-integration-tests/suites/bundled-app/index.ts @@ -0,0 +1,6 @@ +import * as Sentry from '@sentry/bun'; + +Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337' }); + +// eslint-disable-next-line no-console +console.log('Sentry initialized'); diff --git a/dev-packages/bun-integration-tests/suites/bundled-app/test.ts b/dev-packages/bun-integration-tests/suites/bundled-app/test.ts new file mode 100644 index 000000000000..ee3c5c4b8512 --- /dev/null +++ b/dev-packages/bun-integration-tests/suites/bundled-app/test.ts @@ -0,0 +1,32 @@ +import { execFileSync, spawnSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, expect, test } from 'vitest'; + +// Outside the repo, so the bundle has no `node_modules` above it, as in a deployed app. +const runDir = mkdtempSync(join(tmpdir(), 'sentry-bun-bundled-app-')); + +afterAll(() => { + rmSync(runDir, { recursive: true, force: true }); +}); + +// Without `node_modules`, a module lookup starts Bun's auto-install, and Bun exits when it cannot +// create its package cache. See https://github.com/getsentry/sentry-javascript/issues/25540 +test('a bundled app initializes when Bun cannot create its package cache', () => { + const app = join(runDir, 'app.mjs'); + execFileSync('bun', ['build', join(__dirname, 'index.ts'), '--target=bun', '--format=esm', `--outfile=${app}`]); + // Files where Bun creates its cache directories, so that this also fails for the root user. + writeFileSync(join(runDir, 'node_modules'), ''); + writeFileSync(join(runDir, 'cache'), ''); + + const result = spawnSync('bun', [app], { + cwd: runDir, + encoding: 'utf8', + env: { ...process.env, BUN_INSTALL_CACHE_DIR: join(runDir, 'cache', 'bun') }, + }); + + expect(result.stderr).not.toContain('unable to write files'); + expect(result.stdout).toContain('Sentry initialized'); + expect(result.status).toBe(0); +}); From 7fbb96f4160d8dd580efe0a6f3f9067ce84da44e Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 20 Oct 2026 13:27:25 +0200 Subject: [PATCH 3/5] fix comment --- .../test-applications/hono-4-legacy/tests/multi-fetch.test.ts | 4 +--- .../test-applications/hono-4/tests/multi-fetch.test.ts | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts index 7db1a77bdab8..e95f2336e463 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts @@ -360,9 +360,7 @@ test.describe('multi-fetch: internal .request() calls between sub-apps', () => { }); test('error from failed internal fetch is correlated with the storefront trace', async ({ baseURL }) => { - // Use a param unique to this test: the `no error status` test above fetches `/ghost` too, and its - // identical `Failed to fetch product: ghost` error would otherwise be dropped by the Dedupe - // integration, so this test's `waitForError` would never fire. + // Params are unique in each test: ghost` error would otherwise be dropped by the Dedupe integration const errorPromise = waitForError(APP_NAME, event => { return event.exception?.values?.[0]?.value === 'Failed to fetch product: phantom'; }); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts index b98a7e6df7ee..701195ad825a 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts @@ -365,9 +365,7 @@ test.describe('multi-fetch: internal .request() calls between sub-apps', () => { }); test('error from failed internal fetch is correlated with the storefront trace', async ({ baseURL }) => { - // Use a param unique to this test: the `no error status` test above fetches `/ghost` too, and its - // identical `Failed to fetch product: ghost` error would otherwise be dropped by the Dedupe - // integration, so this test's `waitForError` would never fire. + // Params are unique in each test: ghost` error would otherwise be dropped by the Dedupe integration const errorPromise = waitForError(APP_NAME, event => { return event.exception?.values?.[0]?.value === 'Failed to fetch product: phantom'; }); From 171a6a9ce4a5058ecb7661baaa2ca5aac840e235 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:35:54 +0200 Subject: [PATCH 4/5] improve comments --- .../suites/bundled-app/test.ts | 8 ++++---- .../hono-4-legacy/tests/multi-fetch.test.ts | 2 +- .../hono-4-legacy/tests/tracing.test.ts | 2 +- .../hono-4/tests/multi-fetch.test.ts | 2 +- .../hono-4/tests/tracing.test.ts | 7 +++---- .../hono/test/shared/middlewareHandlers.test.ts | 2 +- .../test/shared/requestConnInfoAttributes.ts | 5 +---- .../src/integrations/hono/connInfo.ts | 17 ++++++++--------- .../integrations/hono/createHonoMiddleware.ts | 2 +- .../src/integrations/hono/middlewareHandlers.ts | 7 +++---- 10 files changed, 24 insertions(+), 30 deletions(-) diff --git a/dev-packages/bun-integration-tests/suites/bundled-app/test.ts b/dev-packages/bun-integration-tests/suites/bundled-app/test.ts index ee3c5c4b8512..6684a9af0561 100644 --- a/dev-packages/bun-integration-tests/suites/bundled-app/test.ts +++ b/dev-packages/bun-integration-tests/suites/bundled-app/test.ts @@ -4,19 +4,19 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterAll, expect, test } from 'vitest'; -// Outside the repo, so the bundle has no `node_modules` above it, as in a deployed app. +// Outside the repo, so no `node_modules` is above the bundle, as in a deployed app. const runDir = mkdtempSync(join(tmpdir(), 'sentry-bun-bundled-app-')); afterAll(() => { rmSync(runDir, { recursive: true, force: true }); }); -// Without `node_modules`, a module lookup starts Bun's auto-install, and Bun exits when it cannot -// create its package cache. See https://github.com/getsentry/sentry-javascript/issues/25540 +// Without `node_modules`, a module lookup starts Bun's auto-install, which exits the process when it +// cannot create its cache: https://github.com/getsentry/sentry-javascript/issues/25540 test('a bundled app initializes when Bun cannot create its package cache', () => { const app = join(runDir, 'app.mjs'); execFileSync('bun', ['build', join(__dirname, 'index.ts'), '--target=bun', '--format=esm', `--outfile=${app}`]); - // Files where Bun creates its cache directories, so that this also fails for the root user. + // Files block the cache directories. Unlike file permissions, this also works for root. writeFileSync(join(runDir, 'node_modules'), ''); writeFileSync(join(runDir, 'cache'), ''); diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts index e95f2336e463..ec5182e68aff 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/multi-fetch.test.ts @@ -360,7 +360,7 @@ test.describe('multi-fetch: internal .request() calls between sub-apps', () => { }); test('error from failed internal fetch is correlated with the storefront trace', async ({ baseURL }) => { - // Params are unique in each test: ghost` error would otherwise be dropped by the Dedupe integration + // Not `ghost`: Dedupe would drop this error as a repeat of the error in the test above. const errorPromise = waitForError(APP_NAME, event => { return event.exception?.values?.[0]?.value === 'Failed to fetch product: phantom'; }); diff --git a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/tracing.test.ts b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/tracing.test.ts index 476eb6e7ab3a..0750effa6425 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/tracing.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4-legacy/tests/tracing.test.ts @@ -49,7 +49,7 @@ test('attaches HTTP connection info to the server span', async ({ baseURL, page expect(data['network.peer.address']?.value).toBe(data['client.address']?.value); if (RUNTIME !== 'node' && RUNTIME !== 'deno') { - // Only set by the Node HTTP server span and read from Deno's `remoteAddr` + // Only the Node HTTP server span and Deno's `remoteAddr` give a transport. expect(data['network.transport']?.value).toBeUndefined(); } else { expect(data['network.transport']?.value).toMatch(/tcp/); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts index 701195ad825a..301a39b848ab 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts @@ -365,7 +365,7 @@ test.describe('multi-fetch: internal .request() calls between sub-apps', () => { }); test('error from failed internal fetch is correlated with the storefront trace', async ({ baseURL }) => { - // Params are unique in each test: ghost` error would otherwise be dropped by the Dedupe integration + // Not `ghost`: Dedupe would drop this error as a repeat of the error in the test above. const errorPromise = waitForError(APP_NAME, event => { return event.exception?.values?.[0]?.value === 'Failed to fetch product: phantom'; }); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts index cb8c4789f361..0f9be7f5e52b 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts @@ -6,7 +6,7 @@ const anyString = expect.any(String) as unknown; const anyNumber = expect.any(Number) as unknown; const ipvType = expect.stringMatching(/^ipv[46]$/) as unknown; -// Per-runtime `http.server` connection-info expectations. Each runtime object that Hono holds exposes +// Per-runtime `http.server` connection-info expectations. Each runtime exposes // a different set of fields, so the expected attribute values are declared here once (keyed by // runtime) instead of branching inside the tests. `undefined` means the attribute must be absent. // Relational checks (`network.peer.*` mirroring `client.*`) are asserted in the tests, since they @@ -45,7 +45,7 @@ const CONN_INFO: Record; baseline: Rec baseline: { 'client.port': anyNumber, 'network.type': ipvType }, }, deno: { - // Only Deno's `remoteAddr` exposes `network.transport`. + // `network.transport` comes from Deno's `remoteAddr`. added: { 'client.address': anyString, 'client.port': anyNumber, 'network.transport': expect.stringMatching(/tcp/) }, baseline: { 'server.address': 'localhost', @@ -55,8 +55,7 @@ const CONN_INFO: Record; baseline: Rec }, }, cloudflare: { - // Cloudflare Workers expose only the client address (the `cf-connecting-ip` header): no port, - // address family, or transport. + // Workers give only the client address (the `cf-connecting-ip` header). added: { 'client.address': anyString, 'client.port': undefined, diff --git a/packages/hono/test/shared/middlewareHandlers.test.ts b/packages/hono/test/shared/middlewareHandlers.test.ts index 1cc1bf745b11..83659941b7e4 100644 --- a/packages/hono/test/shared/middlewareHandlers.test.ts +++ b/packages/hono/test/shared/middlewareHandlers.test.ts @@ -354,7 +354,7 @@ describe('requestHandler — connection info (via createHonoRequestMiddleware)', it('does not pair the socket port with a client.address that is already on the root span', async () => { mockUserInfo(true); - // The runtime SDK read `client.address` from a forwarding header and knows no client port. + // Set by the runtime SDK from a forwarding header, without a port. rootSpan.setAttribute('client.address', '198.51.100.7'); await runMiddleware(createNodeContext({ remoteAddress: '10.0.0.1', remotePort: 54321 })); diff --git a/packages/hono/test/shared/requestConnInfoAttributes.ts b/packages/hono/test/shared/requestConnInfoAttributes.ts index b54fe80e6587..e4a6afe8763c 100644 --- a/packages/hono/test/shared/requestConnInfoAttributes.ts +++ b/packages/hono/test/shared/requestConnInfoAttributes.ts @@ -2,10 +2,7 @@ import * as SentryCore from '@sentry/core'; import { Hono, type MiddlewareHandler } from 'hono'; import { vi } from 'vitest'; -/** - * Sends one request through `middleware` inside a root span, with `userInfo` collection on, and - * gives the root span attributes. `env` is what the runtime adapter passes to `app.fetch`. - */ +/** Sends a request through `middleware` in a root span, with `userInfo` on, and returns the span attributes. */ export async function requestConnInfoAttributes( middleware: MiddlewareHandler, { env, headers }: { env?: unknown; headers?: Record }, diff --git a/packages/server-utils/src/integrations/hono/connInfo.ts b/packages/server-utils/src/integrations/hono/connInfo.ts index ac72f2154363..5d1a377a37cb 100644 --- a/packages/server-utils/src/integrations/hono/connInfo.ts +++ b/packages/server-utils/src/integrations/hono/connInfo.ts @@ -10,10 +10,8 @@ export interface ConnInfo { addressType?: string; } -// Structural shapes of what each runtime adapter passes to `app.fetch` as `c.env`. They copy the reads of -// Hono's own `getConnInfo` helpers, which moved out of `hono` in Hono 5. -// The readers cast `c.env` to these shapes without validation: the caller selects the runtime, the runtime -// defines the shape, and `readConnInfo` catches a read that throws. +// What the runtime adapters pass to `app.fetch` as `c.env`. The readers copy Hono's `getConnInfo` helpers, +// which Hono 5 moved out of `hono`. The casts are unchecked, and `readConnInfo` catches a read that throws. type NodeBindings = { incoming?: { socket?: { remoteAddress?: string; remotePort?: number; remoteFamily?: string } }; }; @@ -23,6 +21,7 @@ type BunServer = { type DenoServeHandlerInfo = { remoteAddr?: { hostname?: string; port?: number; transport?: string } }; const READERS: Record ConnInfo | undefined> = { + // https://github.com/honojs/node-server/blob/v2.1.5/src/conninfo.ts node: context => { const env = context.env as (NodeBindings & { server?: NodeBindings }) | undefined; const socket = (env?.server ?? env)?.incoming?.socket; @@ -30,28 +29,28 @@ const READERS: Record ConnInfo | undefined> = ? { address: socket.remoteAddress, port: socket.remotePort, addressType: socket.remoteFamily } : undefined; }, + // https://github.com/honojs/hono/blob/v4.13.13/src/adapter/bun/conninfo.ts bun: context => { const env = context.env as (BunServer & { server?: BunServer }) | undefined; const info = (env?.server ?? env)?.requestIP?.(context.req.raw); return info ? { address: info.address, port: info.port, addressType: info.family } : undefined; }, + // https://github.com/honojs/hono/blob/v4.13.13/src/adapter/deno/conninfo.ts deno: context => { const remoteAddr = (context.env as DenoServeHandlerInfo | undefined)?.remoteAddr; return remoteAddr ? { address: remoteAddr.hostname, port: remoteAddr.port, transport: remoteAddr.transport } : undefined; }, - // On Cloudflare `c.env` holds the user's bindings, so only the request is read. + // `c.env` holds the user's bindings here, so only the header is read. + // https://github.com/honojs/hono/blob/v4.13.13/src/adapter/cloudflare-workers/conninfo.ts cloudflare: context => { const address = context.req.raw.headers.get('cf-connecting-ip'); return address ? { address } : undefined; }, }; -/** - * Reads the connection info from the runtime objects that Hono holds for the request. - * Gives `undefined` when the runtime object is missing or a read throws. - */ +/** Gives `undefined` when the runtime object is missing or a read throws. */ export function readConnInfo(context: Context, runtime: HonoRuntime): ConnInfo | undefined { try { return READERS[runtime](context); diff --git a/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts b/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts index 24ffcfdb00aa..2540fc059871 100644 --- a/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts +++ b/packages/server-utils/src/integrations/hono/createHonoMiddleware.ts @@ -17,7 +17,7 @@ const HONO_REQUEST_HANDLED = Symbol.for('sentry.hono.requestHandled'); const HONO_SHOULD_HANDLE_ERROR = Symbol.for('sentry.hono.shouldHandleError'); export interface CreateHonoRequestMiddlewareOptions { - /** The runtime that serves the app. It selects where the connection info is read from. */ + /** Selects where the connection info is read from. */ runtime: HonoRuntime; /** Static `shouldHandleError` callback (Node/Bun/Deno). */ diff --git a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts index 53909ffaebe5..d1fbeddab8cf 100644 --- a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts +++ b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts @@ -48,9 +48,8 @@ function getCurrentIsolationScope(): Scope { * Adds HTTP connection info (client IP, port, transport, address type) from the runtime to the root * (server) span and the isolation scope. * - * Attributes that are already on the root span are kept: the runtime SDK sets them first and reads - * forwarding headers, while the Node, Bun and Deno runtime objects only know the socket peer (a proxy, - * if there is one). + * Keeps attributes that the runtime SDK already set: it reads forwarding headers, while the runtime + * objects only know the socket peer, which can be a proxy. */ function setConnInfoAttributes(context: Context, runtime: HonoRuntime, isolationScope: Scope): void { const activeSpan = getActiveSpan(); @@ -70,7 +69,7 @@ function setConnInfoAttributes(context: Context, runtime: HonoRuntime, isolation const rootSpan = getRootSpan(activeSpan); const existingAttributes = spanToJSON(rootSpan).attributes ?? {}; - // A port only identifies the client together with its address: behind a proxy, the socket port is the proxy's. + // Pair `client.port` only with the socket address: behind a proxy, the socket port is the proxy's. const writesClientAddress = ipAddress !== undefined && existingAttributes['client.address'] === undefined; rootSpan.setAttributes( onlyNewAttributes( From fa1d4ed60a9fa6d1b712afae781c532d17976e39 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:47:26 +0200 Subject: [PATCH 5/5] use forwarded ip for user and conventions constants --- packages/hono/test/bun/middleware.test.ts | 20 +++-- .../hono/test/cloudflare/middleware.test.ts | 20 +++-- packages/hono/test/deno/middleware.test.ts | 20 +++-- packages/hono/test/node/middleware.test.ts | 20 +++-- .../test/shared/middlewareHandlers.test.ts | 90 ++++++++++++------- .../integrations/hono/middlewareHandlers.ts | 30 +++++-- 6 files changed, 134 insertions(+), 66 deletions(-) diff --git a/packages/hono/test/bun/middleware.test.ts b/packages/hono/test/bun/middleware.test.ts index 1e126529c172..d5668683836f 100644 --- a/packages/hono/test/bun/middleware.test.ts +++ b/packages/hono/test/bun/middleware.test.ts @@ -1,3 +1,11 @@ +import { + CLIENT_ADDRESS, + CLIENT_PORT, + NETWORK_PEER_ADDRESS, + NETWORK_PEER_PORT, + NETWORK_TRANSPORT, + NETWORK_TYPE, +} from '@sentry/conventions/attributes'; import * as SentryCore from '@sentry/core'; import { SDK_VERSION } from '@sentry/core'; import { Hono } from 'hono'; @@ -258,11 +266,11 @@ describe('Hono Bun Middleware — connection info', () => { const attributes = await requestConnInfoAttributes(middleware, { env }); - expect(attributes['client.address']).toBe('::1'); - expect(attributes['client.port']).toBe(54321); - expect(attributes['network.peer.address']).toBe('::1'); - expect(attributes['network.peer.port']).toBe(54321); - expect(attributes['network.type']).toBe('ipv6'); - expect(attributes).not.toHaveProperty('network.transport'); + expect(attributes[CLIENT_ADDRESS]).toBe('::1'); + expect(attributes[CLIENT_PORT]).toBe(54321); + expect(attributes[NETWORK_PEER_ADDRESS]).toBe('::1'); + expect(attributes[NETWORK_PEER_PORT]).toBe(54321); + expect(attributes[NETWORK_TYPE]).toBe('ipv6'); + expect(attributes).not.toHaveProperty(NETWORK_TRANSPORT); }); }); diff --git a/packages/hono/test/cloudflare/middleware.test.ts b/packages/hono/test/cloudflare/middleware.test.ts index 08a7ff1be619..12877986e5a7 100644 --- a/packages/hono/test/cloudflare/middleware.test.ts +++ b/packages/hono/test/cloudflare/middleware.test.ts @@ -1,3 +1,11 @@ +import { + CLIENT_ADDRESS, + CLIENT_PORT, + NETWORK_PEER_ADDRESS, + NETWORK_PEER_PORT, + NETWORK_TRANSPORT, + NETWORK_TYPE, +} from '@sentry/conventions/attributes'; import * as SentryCloudflare from '@sentry/cloudflare'; import * as SentryCore from '@sentry/core'; import { SDK_VERSION } from '@sentry/core'; @@ -206,11 +214,11 @@ describe('Hono Cloudflare Middleware — connection info', () => { const attributes = await requestConnInfoAttributes(middleware, { headers }); - expect(attributes['client.address']).toBe('203.0.113.5'); - expect(attributes['network.peer.address']).toBe('203.0.113.5'); - expect(attributes).not.toHaveProperty('client.port'); - expect(attributes).not.toHaveProperty('network.peer.port'); - expect(attributes).not.toHaveProperty('network.type'); - expect(attributes).not.toHaveProperty('network.transport'); + expect(attributes[CLIENT_ADDRESS]).toBe('203.0.113.5'); + expect(attributes[NETWORK_PEER_ADDRESS]).toBe('203.0.113.5'); + expect(attributes).not.toHaveProperty(CLIENT_PORT); + expect(attributes).not.toHaveProperty(NETWORK_PEER_PORT); + expect(attributes).not.toHaveProperty(NETWORK_TYPE); + expect(attributes).not.toHaveProperty(NETWORK_TRANSPORT); }); }); diff --git a/packages/hono/test/deno/middleware.test.ts b/packages/hono/test/deno/middleware.test.ts index 937af53ba78c..732344bb7786 100644 --- a/packages/hono/test/deno/middleware.test.ts +++ b/packages/hono/test/deno/middleware.test.ts @@ -1,3 +1,11 @@ +import { + CLIENT_ADDRESS, + CLIENT_PORT, + NETWORK_PEER_ADDRESS, + NETWORK_PEER_PORT, + NETWORK_TRANSPORT, + NETWORK_TYPE, +} from '@sentry/conventions/attributes'; import * as SentryCore from '@sentry/core'; import { SDK_VERSION } from '@sentry/core'; import { Hono } from 'hono'; @@ -258,11 +266,11 @@ describe('Hono Deno Middleware — connection info', () => { const attributes = await requestConnInfoAttributes(middleware, { env }); - expect(attributes['client.address']).toBe('127.0.0.1'); - expect(attributes['client.port']).toBe(54321); - expect(attributes['network.peer.address']).toBe('127.0.0.1'); - expect(attributes['network.peer.port']).toBe(54321); - expect(attributes['network.transport']).toBe('tcp'); - expect(attributes).not.toHaveProperty('network.type'); + expect(attributes[CLIENT_ADDRESS]).toBe('127.0.0.1'); + expect(attributes[CLIENT_PORT]).toBe(54321); + expect(attributes[NETWORK_PEER_ADDRESS]).toBe('127.0.0.1'); + expect(attributes[NETWORK_PEER_PORT]).toBe(54321); + expect(attributes[NETWORK_TRANSPORT]).toBe('tcp'); + expect(attributes).not.toHaveProperty(NETWORK_TYPE); }); }); diff --git a/packages/hono/test/node/middleware.test.ts b/packages/hono/test/node/middleware.test.ts index a3d408cae92d..8900556d71fa 100644 --- a/packages/hono/test/node/middleware.test.ts +++ b/packages/hono/test/node/middleware.test.ts @@ -1,3 +1,11 @@ +import { + CLIENT_ADDRESS, + CLIENT_PORT, + NETWORK_PEER_ADDRESS, + NETWORK_PEER_PORT, + NETWORK_TRANSPORT, + NETWORK_TYPE, +} from '@sentry/conventions/attributes'; import * as SentryCore from '@sentry/core'; import { Hono } from 'hono'; import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest'; @@ -240,11 +248,11 @@ describe('Hono Node Middleware — connection info', () => { const attributes = await requestConnInfoAttributes(middleware, { env }); - expect(attributes['client.address']).toBe('203.0.113.5'); - expect(attributes['client.port']).toBe(54321); - expect(attributes['network.peer.address']).toBe('203.0.113.5'); - expect(attributes['network.peer.port']).toBe(54321); - expect(attributes['network.type']).toBe('ipv4'); - expect(attributes).not.toHaveProperty('network.transport'); + expect(attributes[CLIENT_ADDRESS]).toBe('203.0.113.5'); + expect(attributes[CLIENT_PORT]).toBe(54321); + expect(attributes[NETWORK_PEER_ADDRESS]).toBe('203.0.113.5'); + expect(attributes[NETWORK_PEER_PORT]).toBe(54321); + expect(attributes[NETWORK_TYPE]).toBe('ipv4'); + expect(attributes).not.toHaveProperty(NETWORK_TRANSPORT); }); }); diff --git a/packages/hono/test/shared/middlewareHandlers.test.ts b/packages/hono/test/shared/middlewareHandlers.test.ts index 83659941b7e4..fb7d16699d07 100644 --- a/packages/hono/test/shared/middlewareHandlers.test.ts +++ b/packages/hono/test/shared/middlewareHandlers.test.ts @@ -1,4 +1,13 @@ -import { HTTP_ROUTE, SENTRY_SEGMENT_NAME_SOURCE } from '@sentry/conventions/attributes'; +import { + CLIENT_ADDRESS, + CLIENT_PORT, + HTTP_ROUTE, + NETWORK_PEER_ADDRESS, + NETWORK_PEER_PORT, + NETWORK_TRANSPORT, + NETWORK_TYPE, + SENTRY_SEGMENT_NAME_SOURCE, +} from '@sentry/conventions/attributes'; import * as SentryCore from '@sentry/core'; import { createHonoRequestMiddleware } from '@sentry/server-utils'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -224,8 +233,10 @@ describe('requestHandler — connection info (via createHonoRequestMiddleware)', } // The `c.env` that `@hono/node-server` passes to `app.fetch`. - function createNodeContext(socket: Record): object { - return Object.assign(createMockContext(200), { env: { incoming: { socket } } }); + function createNodeContext(socket: Record, headers?: Record): object { + const context = createMockContext(200) as { req: { raw: Request } }; + context.req.raw = new Request('http://localhost/test', { headers }); + return Object.assign(context, { env: { incoming: { socket } } }); } it('sets the peer port and address type regardless of userInfo', async () => { @@ -234,8 +245,8 @@ describe('requestHandler — connection info (via createHonoRequestMiddleware)', await runMiddleware(createNodeContext({ remoteAddress: '203.0.113.5', remotePort: 54321, remoteFamily: 'IPv4' })); const attributes = getRootSpanAttributes(); - expect(attributes['network.peer.port']).toBe(54321); - expect(attributes['network.type']).toBe('ipv4'); + expect(attributes[NETWORK_PEER_PORT]).toBe(54321); + expect(attributes[NETWORK_TYPE]).toBe('ipv4'); }); it('sets IP-bearing attributes and user.ip_address when userInfo is true', async () => { @@ -244,10 +255,10 @@ describe('requestHandler — connection info (via createHonoRequestMiddleware)', await runMiddleware(createNodeContext({ remoteAddress: '203.0.113.5', remotePort: 443, remoteFamily: 'IPv6' })); const attributes = getRootSpanAttributes(); - expect(attributes['client.address']).toBe('203.0.113.5'); - expect(attributes['client.port']).toBe(443); - expect(attributes['network.peer.address']).toBe('203.0.113.5'); - expect(attributes['network.type']).toBe('ipv6'); + expect(attributes[CLIENT_ADDRESS]).toBe('203.0.113.5'); + expect(attributes[CLIENT_PORT]).toBe(443); + expect(attributes[NETWORK_PEER_ADDRESS]).toBe('203.0.113.5'); + expect(attributes[NETWORK_TYPE]).toBe('ipv6'); expect(mockSetUser).toHaveBeenCalledWith({ ip_address: '203.0.113.5' }); }); @@ -267,27 +278,29 @@ describe('requestHandler — connection info (via createHonoRequestMiddleware)', it('omits IP-bearing attributes when userInfo is false', async () => { mockUserInfo(false); - await runMiddleware(createNodeContext({ remoteAddress: '203.0.113.5', remotePort: 8080 })); + await runMiddleware( + createNodeContext({ remoteAddress: '203.0.113.5', remotePort: 8080 }, { 'x-forwarded-for': '198.51.100.7' }), + ); const attributes = getRootSpanAttributes(); - expect(attributes).not.toHaveProperty('client.address'); - expect(attributes).not.toHaveProperty('client.port'); - expect(attributes).not.toHaveProperty('network.peer.address'); + expect(attributes).not.toHaveProperty(CLIENT_ADDRESS); + expect(attributes).not.toHaveProperty(CLIENT_PORT); + expect(attributes).not.toHaveProperty(NETWORK_PEER_ADDRESS); expect(mockSetUser).not.toHaveBeenCalled(); // Non-PII data is still recorded. - expect(attributes['network.peer.port']).toBe(8080); + expect(attributes[NETWORK_PEER_PORT]).toBe(8080); }); it('does not write the attributes that the runtime does not give', async () => { mockUserInfo(true); - rootSpan.setAttribute('network.transport', 'tcp'); + rootSpan.setAttribute(NETWORK_TRANSPORT, 'tcp'); await runMiddleware(createNodeContext({ remotePort: 54321 })); const attributes = getRootSpanAttributes(); - expect(attributes['network.transport']).toBe('tcp'); - expect(attributes).not.toHaveProperty('client.address'); - expect(attributes).not.toHaveProperty('network.type'); + expect(attributes[NETWORK_TRANSPORT]).toBe('tcp'); + expect(attributes).not.toHaveProperty(CLIENT_ADDRESS); + expect(attributes).not.toHaveProperty(NETWORK_TYPE); expect(mockSetUser).not.toHaveBeenCalled(); }); @@ -301,7 +314,7 @@ describe('requestHandler — connection info (via createHonoRequestMiddleware)', const context = Object.assign(createMockContext(200), { env: { incoming } }); await expect(runMiddleware(context)).resolves.toBeUndefined(); - expect(getRootSpanAttributes()).not.toHaveProperty('client.port'); + expect(getRootSpanAttributes()).not.toHaveProperty(CLIENT_PORT); expect(mockSetUser).not.toHaveBeenCalled(); }); @@ -312,8 +325,8 @@ describe('requestHandler — connection info (via createHonoRequestMiddleware)', await runMiddleware(createNodeContext({ remoteAddress: '203.0.113.5', remotePort: 443 })); const attributes = getRootSpanAttributes(); - expect(attributes).not.toHaveProperty('client.address'); - expect(attributes).not.toHaveProperty('network.peer.port'); + expect(attributes).not.toHaveProperty(CLIENT_ADDRESS); + expect(attributes).not.toHaveProperty(NETWORK_PEER_PORT); expect(mockSetUser).not.toHaveBeenCalled(); }); @@ -323,8 +336,8 @@ describe('requestHandler — connection info (via createHonoRequestMiddleware)', await runMiddleware(createMockContext(200)); const attributes = getRootSpanAttributes(); - expect(attributes).not.toHaveProperty('client.port'); - expect(attributes).not.toHaveProperty('client.address'); + expect(attributes).not.toHaveProperty(CLIENT_PORT); + expect(attributes).not.toHaveProperty(CLIENT_ADDRESS); expect(mockSetUser).not.toHaveBeenCalled(); }); @@ -336,32 +349,43 @@ describe('requestHandler — connection info (via createHonoRequestMiddleware)', await runMiddleware(context, { runtime: 'deno' }); - expect(getRootSpanAttributes()['network.transport']).toBe('tcp'); + expect(getRootSpanAttributes()[NETWORK_TRANSPORT]).toBe('tcp'); }); it('keeps attributes that are already on the root span', async () => { mockUserInfo(true); - rootSpan.setAttributes({ 'client.address': '198.51.100.7', 'client.port': 1234 }); + rootSpan.setAttributes({ [CLIENT_ADDRESS]: '198.51.100.7', [CLIENT_PORT]: 1234 }); await runMiddleware(createNodeContext({ remoteAddress: '10.0.0.1', remotePort: 54321 })); const attributes = getRootSpanAttributes(); - expect(attributes['client.address']).toBe('198.51.100.7'); - expect(attributes['client.port']).toBe(1234); - expect(attributes['network.peer.address']).toBe('10.0.0.1'); - expect(attributes['network.peer.port']).toBe(54321); + expect(attributes[CLIENT_ADDRESS]).toBe('198.51.100.7'); + expect(attributes[CLIENT_PORT]).toBe(1234); + expect(attributes[NETWORK_PEER_ADDRESS]).toBe('10.0.0.1'); + expect(attributes[NETWORK_PEER_PORT]).toBe(54321); }); it('does not pair the socket port with a client.address that is already on the root span', async () => { mockUserInfo(true); // Set by the runtime SDK from a forwarding header, without a port. - rootSpan.setAttribute('client.address', '198.51.100.7'); + rootSpan.setAttribute(CLIENT_ADDRESS, '198.51.100.7'); await runMiddleware(createNodeContext({ remoteAddress: '10.0.0.1', remotePort: 54321 })); const attributes = getRootSpanAttributes(); - expect(attributes['client.address']).toBe('198.51.100.7'); - expect(attributes).not.toHaveProperty('client.port'); - expect(attributes['network.peer.port']).toBe(54321); + expect(attributes[CLIENT_ADDRESS]).toBe('198.51.100.7'); + expect(attributes).not.toHaveProperty(CLIENT_PORT); + expect(attributes[NETWORK_PEER_PORT]).toBe(54321); + }); + + it('sets user.ip_address from a forwarding header before the socket address', async () => { + mockUserInfo(true); + + await runMiddleware( + createNodeContext({ remoteAddress: '10.0.0.1', remotePort: 54321 }, { 'x-forwarded-for': '198.51.100.7' }), + ); + + expect(mockSetUser).toHaveBeenCalledWith({ ip_address: '198.51.100.7' }); + expect(getRootSpanAttributes()[NETWORK_PEER_ADDRESS]).toBe('10.0.0.1'); }); }); diff --git a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts index d1fbeddab8cf..6ca799b3e340 100644 --- a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts +++ b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts @@ -10,15 +10,25 @@ import { type Scope, type SpanAttributes, spanToJSON, + winterCGHeadersToDict, winterCGRequestToRequestData, } from '@sentry/core'; +import { getClientIPAddress } from '@sentry/core/server'; import type { Context } from './honoTypes'; import { type HonoRuntime, readConnInfo } from './connInfo'; import { hasFetchEvent } from './hono-context'; import { defaultShouldHandleError } from './defaultShouldHandleError'; import { resolveRouteName } from './resolveRouteName'; import { type SentryHonoMiddlewareOptions } from './types'; -import { HTTP_ROUTE } from '@sentry/conventions/attributes'; +import { + CLIENT_ADDRESS, + CLIENT_PORT, + HTTP_ROUTE, + NETWORK_PEER_ADDRESS, + NETWORK_PEER_PORT, + NETWORK_TRANSPORT, + NETWORK_TYPE, +} from '@sentry/conventions/attributes'; /** * Request handler for Hono framework @@ -70,23 +80,25 @@ function setConnInfoAttributes(context: Context, runtime: HonoRuntime, isolation const rootSpan = getRootSpan(activeSpan); const existingAttributes = spanToJSON(rootSpan).attributes ?? {}; // Pair `client.port` only with the socket address: behind a proxy, the socket port is the proxy's. - const writesClientAddress = ipAddress !== undefined && existingAttributes['client.address'] === undefined; + const writesClientAddress = ipAddress !== undefined && existingAttributes[CLIENT_ADDRESS] === undefined; rootSpan.setAttributes( onlyNewAttributes( { - 'client.port': writesClientAddress ? port : undefined, - 'network.peer.port': port, - 'network.transport': transport, - 'network.type': addressType?.toLowerCase(), - 'client.address': ipAddress, - 'network.peer.address': ipAddress, + [CLIENT_PORT]: writesClientAddress ? port : undefined, + [NETWORK_PEER_PORT]: port, + [NETWORK_TRANSPORT]: transport, + [NETWORK_TYPE]: addressType?.toLowerCase(), + [CLIENT_ADDRESS]: ipAddress, + [NETWORK_PEER_ADDRESS]: ipAddress, }, existingAttributes, ), ); if (ipAddress) { - isolationScope.setUser({ ...isolationScope.getUser(), ip_address: ipAddress }); + // Like `RequestData`, prefer the forwarding header: behind a proxy, the socket address is the proxy's. + const userIpAddress = getClientIPAddress(winterCGHeadersToDict(context.req.raw.headers)) || ipAddress; + isolationScope.setUser({ ...isolationScope.getUser(), ip_address: userIpAddress }); } }