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..6684a9af0561 --- /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 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, 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 block the cache directories. Unlike file permissions, this also works for root. + 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); +}); 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..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,8 +360,9 @@ test.describe('multi-fetch: internal .request() calls between sub-apps', () => { }); test('error from failed internal fetch is correlated with the storefront trace', async ({ baseURL }) => { + // 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: ghost'; + return event.exception?.values?.[0]?.value === 'Failed to fetch product: phantom'; }); const segmentPromise = waitForStreamedSpan( @@ -372,7 +373,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..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 @@ -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 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/); @@ -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/multi-fetch.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/multi-fetch.test.ts index b98a7e6df7ee..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,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. + // 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 9350259d7e5c..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's `getConnInfo` helper 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 @@ -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`. + // `network.transport` comes from Deno's `remoteAddr`. added: { 'client.address': anyString, 'client.port': anyNumber, 'network.transport': expect.stringMatching(/tcp/) }, baseline: { 'server.address': 'localhost', @@ -55,11 +55,9 @@ 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. + // Workers give only the client address (the `cf-connecting-ip` header). 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 ce561d1101ac..e5748e8d0051 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.8.0", "@sentry/cloudflare": "11.8.0", "@sentry/deno": "11.8.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": "^20.19.0", "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..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'; @@ -5,17 +13,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 +257,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..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'; @@ -5,6 +13,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 +206,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..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'; @@ -5,17 +13,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 +257,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..8900556d71fa 100644 --- a/packages/hono/test/node/middleware.test.ts +++ b/packages/hono/test/node/middleware.test.ts @@ -1,17 +1,22 @@ +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'; 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 +237,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..fb7d16699d07 100644 --- a/packages/hono/test/shared/middlewareHandlers.test.ts +++ b/packages/hono/test/shared/middlewareHandlers.test.ts @@ -1,7 +1,16 @@ -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 { 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 +20,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 +46,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 +71,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 +208,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 +232,41 @@ 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, 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 () => { 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 +277,115 @@ 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 }, { 'x-forwarded-for': '198.51.100.7' }), + ); - 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); + // 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 })); + + 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); + }); + + 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/hono/test/shared/requestConnInfoAttributes.ts b/packages/hono/test/shared/requestConnInfoAttributes.ts new file mode 100644 index 000000000000..e4a6afe8763c --- /dev/null +++ b/packages/hono/test/shared/requestConnInfoAttributes.ts @@ -0,0 +1,25 @@ +import * as SentryCore from '@sentry/core'; +import { Hono, type MiddlewareHandler } from 'hono'; +import { vi } from 'vitest'; + +/** 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 }, +): 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..5d1a377a37cb --- /dev/null +++ b/packages/server-utils/src/integrations/hono/connInfo.ts @@ -0,0 +1,60 @@ +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; +} + +// 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 } }; +}; +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> = { + // 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; + return socket + ? { 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; + }, + // `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; + }, +}; + +/** 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..2540fc059871 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; + /** 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..6ca799b3e340 100644 --- a/packages/server-utils/src/integrations/hono/middlewareHandlers.ts +++ b/packages/server-utils/src/integrations/hono/middlewareHandlers.ts @@ -8,19 +8,32 @@ import { INTERNAL_setSegmentNameSourceIfSegment, updateSpanName, type Scope, + type SpanAttributes, + spanToJSON, + winterCGHeadersToDict, winterCGRequestToRequestData, } from '@sentry/core'; -import type { Context, GetConnInfo } from './honoTypes'; +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 */ -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 +44,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 +55,60 @@ 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. + * + * 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, 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 ?? {}; + // 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( + { + [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 }); } } +/** 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 () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a23875bb2c3e..0a2de0cc42f4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1595,9 +1595,6 @@ importers: '@cloudflare/workers-types': specifier: 4.20250922.0 version: 4.20250922.0 - '@hono/node-server': - specifier: ^2.0.10 - version: 2.0.10(hono@4.13.7) '@types/node': specifier: ^20.19.0 version: 20.19.43