From eebfaa10f70bdf31fda9a3f975fa4d82a2cefc8d Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 8 May 2026 22:05:22 -0700 Subject: [PATCH] feat(core, node): portable Connect integration (#20882) Platform-portable Connect tracing integration in `@sentry/core` (`patchConnectModule`, `setupConnectErrorHandler`), similar to portable Express integration, and rewire the Node SDK's Connect integration to call into it through the otel InstrumentationBase class. Remove OTel-specific span attribute fix-up. Spans created with correct origin (`auto.http.connect`) and op directly in the middleware. --- .../node-connect/tests/transactions.test.ts | 4 +- .../suites/tracing/connect/test.ts | 50 +- .../core/src/integrations/connect/index.ts | 315 ++++++++++++ packages/core/src/server-exports.ts | 2 + .../lib/integrations/connect/index.test.ts | 451 ++++++++++++++++++ .../node/src/integrations/tracing/connect.ts | 109 ++--- 6 files changed, 843 insertions(+), 88 deletions(-) create mode 100644 packages/core/src/integrations/connect/index.ts create mode 100644 packages/core/test/lib/integrations/connect/index.test.ts diff --git a/dev-packages/e2e-tests/test-applications/node-connect/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-connect/tests/transactions.test.ts index 9b06ad052f58..84c5a6019210 100644 --- a/dev-packages/e2e-tests/test-applications/node-connect/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-connect/tests/transactions.test.ts @@ -72,7 +72,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { }, { data: { - 'sentry.origin': 'auto.http.otel.connect', + 'sentry.origin': 'auto.http.connect', 'sentry.op': 'request_handler.connect', 'http.route': '/test-transaction', 'connect.type': 'request_handler', @@ -86,7 +86,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { status: 'ok', timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), - origin: 'auto.http.otel.connect', + origin: 'auto.http.connect', }, ], transaction: 'GET /test-transaction', diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts index 0c37c58f8a4c..bcc8033710d1 100644 --- a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts @@ -14,11 +14,11 @@ describe('connect auto-instrumentation', () => { 'connect.name': '/', 'connect.type': 'request_handler', 'http.route': '/', - 'sentry.origin': 'auto.http.otel.connect', + 'sentry.origin': 'auto.http.connect', 'sentry.op': 'request_handler.connect', }), description: '/', - origin: 'auto.http.otel.connect', + origin: 'auto.http.connect', op: 'request_handler.connect', status: 'ok', }), @@ -36,32 +36,26 @@ describe('connect auto-instrumentation', () => { }, }; - createEsmAndCjsTests( - __dirname, - 'scenario.mjs', - 'instrument.mjs', - (createTestRunner, test) => { - test('should auto-instrument `connect` package.', async () => { - const runner = createTestRunner().expect({ transaction: EXPECTED_TRANSACTION }).start(); - runner.makeRequest('get', '/'); - await runner.completed(); - }); + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createTestRunner, test) => { + test('should auto-instrument `connect` package.', async () => { + const runner = createTestRunner().expect({ transaction: EXPECTED_TRANSACTION }).start(); + runner.makeRequest('get', '/'); + await runner.completed(); + }); - test('should capture errors in `connect` middleware.', async () => { - const runner = createTestRunner().ignore('transaction').expect({ event: EXPECTED_EVENT }).start(); - runner.makeRequest('get', '/error'); - await runner.completed(); - }); + test('should capture errors in `connect` middleware.', async () => { + const runner = createTestRunner().ignore('transaction').expect({ event: EXPECTED_EVENT }).start(); + runner.makeRequest('get', '/error'); + await runner.completed(); + }); - test('should report errored transactions.', async () => { - const runner = createTestRunner() - .ignore('event') - .expect({ transaction: { transaction: 'GET /error' } }) - .start(); - runner.makeRequest('get', '/error'); - await runner.completed(); - }); - }, - { failsOnEsm: true }, - ); + test('should report errored transactions.', async () => { + const runner = createTestRunner() + .ignore('event') + .expect({ transaction: { transaction: 'GET /error' } }) + .start(); + runner.makeRequest('get', '/error'); + await runner.completed(); + }); + }); }); diff --git a/packages/core/src/integrations/connect/index.ts b/packages/core/src/integrations/connect/index.ts new file mode 100644 index 000000000000..5a5168a7e7d3 --- /dev/null +++ b/packages/core/src/integrations/connect/index.ts @@ -0,0 +1,315 @@ +/** + * Platform-portable Connect tracing integration. + * + * @module + * + * This Sentry integration is a derivative work based on the OpenTelemetry + * Connect instrumentation. + * + * + * + * Extended under the terms of the Apache 2.0 license linked below: + * + * ---- + * + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { captureException } from '../../exports'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../../semanticAttributes'; +import { startInactiveSpan } from '../../tracing'; +import { getActiveSpan } from '../../utils/spanUtils'; +import { wrapMethod } from '../../utils/object'; +import { DEBUG_BUILD } from '../../debug-build'; +import { debug } from '../../utils/debug-logger'; +import { getDefaultExport } from '../../utils/get-default-export'; + +export const ANONYMOUS_NAME = 'anonymous'; + +// Symbol used to store the route stack on the request object (non-enumerable) +const _LAYERS_STORE_PROPERTY = Symbol('sentry.connect.request-route-stack'); + +// --- Type definitions --- + +export type ConnectRequest = { + method?: string; + url?: string; +}; + +export type ConnectResponse = { + addListener(event: string, listener: () => void): void; + removeListener(event: string, listener: () => void): void; +}; + +export type ConnectMiddleware = ((...args: unknown[]) => unknown) & { + name?: string; + length: number; +}; + +export type ConnectApp = { + use: (...args: unknown[]) => ConnectApp; + handle: (...args: unknown[]) => unknown; +}; + +// The connect module export is a factory function: connect() returns a ConnectApp +export type ConnectModule = (...args: unknown[]) => ConnectApp; + +export type ConnectModuleExport = + | ConnectModule + | { + default: ConnectModule; + }; + +export interface ConnectIntegrationOptions { + /** + * Optional callback invoked each time a named route handler resolves the + * matched HTTP route. Platform-specific integrations (e.g. Node.js) can use + * this to propagate the resolved route to OTel RPCMetadata. + */ + onRouteResolved?: (route: string) => void; +} + +// --- Internal route stack management --- +// Tracks nested route paths on the request object, mirroring the OTel +// connect instrumentation's approach for building the full HTTP route. + +// `ConnectRequest` is intentionally minimal so it stays compatible with +// downstream consumers compiled under TS 3.8 (which doesn't allow `symbol` +// index signatures). The route stack is stored on the request via a Symbol +// key; we cast through a symbol-indexed type here so the access is typed +// without leaking the symbol-index into the public `ConnectRequest` type. +function getLayers(req: ConnectRequest): string[] | undefined { + return (req as { [k: symbol]: unknown })[_LAYERS_STORE_PROPERTY] as string[] | undefined; +} + +function addNewStackLayer(req: ConnectRequest): () => void { + let layers = getLayers(req); + if (!Array.isArray(layers)) { + layers = []; + Object.defineProperty(req, _LAYERS_STORE_PROPERTY, { + enumerable: false, + value: layers, + }); + } + layers.push('/'); + const stackLength = layers.length; + return () => { + const current = getLayers(req); + if (Array.isArray(current) && current.length === stackLength) { + current.pop(); + } + }; +} + +function replaceCurrentStackRoute(req: ConnectRequest, newRoute: string): void { + if (!newRoute) return; + const layers = getLayers(req); + if (Array.isArray(layers) && layers.length > 0) { + layers.splice(-1, 1, newRoute); + } +} + +// Combines all stack layers into a single route path, deduplicating slashes: +// ['/api/', '/users', '/:id'] => '/api/users/:id' +function generateRoute(req: ConnectRequest): string { + const layers = getLayers(req); + /* v8 ignore start */ + if (!Array.isArray(layers) || layers.length === 0) return '/'; + return layers.reduce((acc: string, sub: string) => acc.replace(/\/+$/, '') + sub); +} + +// --- Middleware patching --- + +function patchMiddleware( + routeName: string, + middleware: ConnectMiddleware, + options?: ConnectIntegrationOptions, +): ConnectMiddleware { + // Error middlewares have 4 arguments: (err, req, res, next) + const isErrorMiddleware = middleware.length === 4; + + function patchedMiddleware(this: unknown, ...args: unknown[]): unknown { + const parentSpan = getActiveSpan(); + if (!parentSpan) { + return middleware.apply(this, args); + } + + const [reqArgIdx, resArgIdx, nextArgIdx] = isErrorMiddleware ? [1, 2, 3] : [0, 1, 2]; + const req = args[reqArgIdx] as ConnectRequest; + const res = args[resArgIdx] as ConnectResponse; + const next = args[nextArgIdx] as ((...a: unknown[]) => unknown) | undefined; + + replaceCurrentStackRoute(req, routeName); + + const isRequestHandler = !!routeName; + const connectType = isRequestHandler ? 'request_handler' : 'middleware'; + const connectName = isRequestHandler ? routeName : middleware.name || ANONYMOUS_NAME; + + if (isRequestHandler) { + options?.onRouteResolved?.(generateRoute(req)); + } + + // Create the span as inactive so the middleware (and anything it calls, + // including the connect error-handler middleware) runs with the original + // parent span — typically the http.server span — as the active span. + // This matches the behavior of OpenTelemetry's connect instrumentation + // and keeps captureException calls inside middleware attached to the + // root server span instead of the per-middleware span. + const span = startInactiveSpan({ + name: connectName, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${connectType}.connect`, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.connect', + 'http.route': routeName.length > 0 ? routeName : '/', + 'connect.type': connectType, + 'connect.name': connectName, + }, + }); + + let spanFinished = false; + + function finishSpan(): void { + if (!spanFinished) { + spanFinished = true; + span.end(); + } + res.removeListener('close', finishSpan); + } + + // End the span when the response closes (handles async middlewares that + // do not call next()) + res.addListener('close', finishSpan); + + if (typeof next === 'function') { + args[nextArgIdx] = function patchedNext(this: unknown, ...nextArgs: unknown[]) { + finishSpan(); + return next.apply(this, nextArgs); + }; + } + + return middleware.apply(this, args); + } + + // Preserve the original function's arity so connect can detect error + // middlewares (length === 4) correctly. + Object.defineProperty(patchedMiddleware, 'length', { + value: middleware.length, + writable: false, + configurable: true, + }); + + return patchedMiddleware; +} + +// --- App patching --- + +/** + * Patch an already-created connect app instance. + */ +export function patchConnectApp(app: ConnectApp, options?: ConnectIntegrationOptions): void { + const originalUse = app.use; + try { + wrapMethod(app, 'use', function patchedUse(this: ConnectApp, ...args: unknown[]) { + // connect.use([route,] middleware) — the route is optional + const middleware = args[args.length - 1] as ConnectMiddleware; + /* v8 ignore start */ + const routeName = args.length > 1 ? String(args[args.length - 2] ?? '') : ''; + args[args.length - 1] = patchMiddleware(routeName, middleware, options); + return originalUse.apply(this, args); + }); + } catch (e) { + DEBUG_BUILD && debug.error('Failed to patch connect use method:', e); + } + + const originalHandle = app.handle; + try { + wrapMethod(app, 'handle', function patchedHandle(this: unknown, ...args: unknown[]) { + // handle(req, res[, out]) — 'out' is the fallback called when no + // middleware matches the request (used for nested apps). + const req = args[0] as ConnectRequest; + const out = args[2]; + const completeStack = addNewStackLayer(req); + if (typeof out === 'function') { + args[2] = function patchedOut(this: unknown, ...outArgs: unknown[]) { + completeStack(); + return (out as (...a: unknown[]) => unknown).apply(this, outArgs); + }; + } + return originalHandle.apply(this, args); + }); + } catch (e) { + DEBUG_BUILD && debug.error('Failed to patch connect handle method:', e); + } +} + +/** + * Wrap the connect factory function so that every app created with it is + * automatically patched. + * + * @example + * ```javascript + * import connect from 'connect'; + * import * as Sentry from '@sentry/node'; // or any SDK that extends core + * + * const patchedConnect = Sentry.patchConnectModule(connect); + * const app = patchedConnect(); + * ``` + */ +export function patchConnectModule( + connectModule: ConnectModuleExport, + options?: ConnectIntegrationOptions, +): ConnectModule { + const connect = getDefaultExport(connectModule); + return function patchedConnect(this: unknown, ...args: unknown[]) { + const app = connect.apply(this, args) as ConnectApp; + patchConnectApp(app, options); + return app; + } as ConnectModule; +} + +// --- Error handler --- + +function connectErrorMiddleware(err: unknown, _req: unknown, _res: unknown, next: (err: unknown) => void): void { + captureException(err, { + mechanism: { + handled: false, + type: 'auto.middleware.connect', + }, + }); + next(err); +} + +/** + * Add a Connect middleware to capture errors to Sentry. + * + * @param app The Connect app to attach the error handler to + * + * @example + * ```javascript + * const Sentry = require('@sentry/node'); + * const connect = require('connect'); + * + * const app = connect(); + * + * Sentry.setupConnectErrorHandler(app); + * + * // Add your connect routes here + * + * app.listen(3000); + * ``` + */ +export function setupConnectErrorHandler(app: { use: (middleware: unknown) => void }): void { + app.use(connectErrorMiddleware); +} diff --git a/packages/core/src/server-exports.ts b/packages/core/src/server-exports.ts index d9d5aa1eda1e..6d61a73ca4c6 100644 --- a/packages/core/src/server-exports.ts +++ b/packages/core/src/server-exports.ts @@ -43,3 +43,5 @@ export type { HttpServerResponse, HttpModuleExport, } from './integrations/http/types'; +export { patchConnectModule, setupConnectErrorHandler } from './integrations/connect/index'; +export type { ConnectIntegrationOptions, ConnectModule } from './integrations/connect/index'; diff --git a/packages/core/test/lib/integrations/connect/index.test.ts b/packages/core/test/lib/integrations/connect/index.test.ts new file mode 100644 index 000000000000..e5d79116b862 --- /dev/null +++ b/packages/core/test/lib/integrations/connect/index.test.ts @@ -0,0 +1,451 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { + ANONYMOUS_NAME, + patchConnectApp, + patchConnectModule, + setupConnectErrorHandler, + type ConnectApp, + type ConnectModule, + type ConnectRequest, + type ConnectResponse, +} from '../../../../src/integrations/connect/index'; + +// --- Mock Sentry core --- + +const activeSpans: ReturnType[] = []; +let mockParentSpan: ReturnType | null = null; + +function makeMockSpan(name = 'span') { + return { + name, + ended: false, + attributes: {} as Record, + setAttributes(attrs: Record) { + Object.assign(this.attributes, attrs); + }, + end() { + this.ended = true; + }, + }; +} + +vi.mock('../../../../src/utils/spanUtils', () => ({ + getActiveSpan: () => mockParentSpan, +})); + +const startInactiveSpanCalls: { options: unknown }[] = []; + +vi.mock('../../../../src/tracing', () => ({ + startInactiveSpan(options: unknown) { + const span = makeMockSpan((options as { name: string }).name); + activeSpans.push(span); + startInactiveSpanCalls.push({ options }); + return span; + }, +})); + +const capturedExceptions: [unknown, unknown][] = []; +vi.mock('../../../../src/exports', () => ({ + captureException(error: unknown, hint: unknown) { + capturedExceptions.push([error, hint]); + return 'eventId'; + }, +})); + +vi.mock('../../../../src/debug-build', () => ({ DEBUG_BUILD: true })); +const debugErrors: [string, unknown][] = []; +vi.mock('../../../../src/utils/debug-logger', () => ({ + debug: { error: (msg: string, e: unknown) => debugErrors.push([msg, e]) }, +})); + +vi.mock('../../../../src/semanticAttributes', () => ({ + SEMANTIC_ATTRIBUTE_SENTRY_OP: 'sentry.op', + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN: 'sentry.origin', +})); + +// --- Helpers --- + +function makeRequest(): ConnectRequest { + return { method: 'GET', url: '/test' }; +} + +function makeResponse(): ConnectResponse & { listeners: Record void)[]> } { + const listeners: Record void)[]> = {}; + return { + listeners, + addListener(event: string, fn: () => void) { + (listeners[event] ??= []).push(fn); + }, + removeListener(event: string, fn: () => void) { + listeners[event] = (listeners[event] ?? []).filter(l => l !== fn); + }, + }; +} + +function makeApp(): ConnectApp & { stack: Array<(...a: unknown[]) => unknown> } { + const stack: Array<(...a: unknown[]) => unknown> = []; + return { + stack, + use(...args: unknown[]) { + stack.push(args[args.length - 1] as (...a: unknown[]) => unknown); + return this; + }, + handle(req: unknown, res: unknown, out?: unknown) { + for (const fn of stack) { + fn(req, res, out ?? (() => undefined)); + } + return undefined; + }, + } as unknown as ConnectApp & { stack: Array<(...a: unknown[]) => unknown> }; +} + +function makeConnectModule(): ConnectModule { + return function connect() { + return makeApp(); + }; +} + +beforeEach(() => { + activeSpans.length = 0; + startInactiveSpanCalls.length = 0; + capturedExceptions.length = 0; + debugErrors.length = 0; + mockParentSpan = makeMockSpan('parent'); +}); + +// --- Tests --- + +describe('patchConnectModule', () => { + it('returns a factory that creates patched apps', () => { + const connect = makeConnectModule(); + const patched = patchConnectModule(connect); + const app = patched(); + expect(typeof app.use).toBe('function'); + expect(typeof app.handle).toBe('function'); + }); + + it('patched factory passes args to original factory', () => { + let receivedArgs: unknown[] = []; + const connect = function (...args: unknown[]) { + receivedArgs = args; + return makeApp(); + }; + const patched = patchConnectModule(connect); + patched('arg1', 'arg2'); + expect(receivedArgs).toStrictEqual(['arg1', 'arg2']); + }); + + it('wraps middleware added via use to create spans', () => { + const connect = makeConnectModule(); + const patched = patchConnectModule(connect); + const app = patched(); + + const req = makeRequest(); + const res = makeResponse(); + let middlewareCalled = false; + const middleware = function (_req: unknown, _res: unknown, n: () => void) { + middlewareCalled = true; + n(); + }; + // Clear inferred name so connect treats it as anonymous + Object.defineProperty(middleware, 'name', { value: '', configurable: true }); + + app.use(middleware as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + + expect(middlewareCalled).toBe(true); + expect(startInactiveSpanCalls).toHaveLength(1); + expect((startInactiveSpanCalls[0]!.options as { name: string }).name).toBe(ANONYMOUS_NAME); + }); +}); + +describe('patchConnectApp', () => { + it('wraps anonymous middleware without a route', () => { + const app = makeApp(); + patchConnectApp(app); + + const req = makeRequest(); + const res = makeResponse(); + let middlewareCalled = false; + // Use a function expression so .name === '' (treated as anonymous) + const middleware = function (_req: unknown, _res: unknown, n: () => void) { + middlewareCalled = true; + n(); + }; + + // Clear inferred name so connect treats it as anonymous + Object.defineProperty(middleware, 'name', { value: '', configurable: true }); + + app.use(middleware as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + expect(middlewareCalled).toBe(true); + + expect(startInactiveSpanCalls).toHaveLength(1); + const spanOptions = startInactiveSpanCalls[0]!.options as Record; + expect(spanOptions['name']).toBe(ANONYMOUS_NAME); + expect((spanOptions['attributes'] as Record)['connect.type']).toBe('middleware'); + expect((spanOptions['attributes'] as Record)['connect.name']).toBe(ANONYMOUS_NAME); + expect((spanOptions['attributes'] as Record)['sentry.op']).toBe('middleware.connect'); + expect((spanOptions['attributes'] as Record)['sentry.origin']).toBe('auto.http.connect'); + }); + + it('uses middleware.name when available for anonymous middleware', () => { + const app = makeApp(); + patchConnectApp(app); + + const req = makeRequest(); + const res = makeResponse(); + function myHandler(_req: unknown, _res: unknown, next: () => void) { + next(); + } + + app.use(myHandler as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + + expect(startInactiveSpanCalls).toHaveLength(1); + expect((startInactiveSpanCalls[0]!.options as { name: string }).name).toBe('myHandler'); + }); + + it('wraps named route handler with routeName', () => { + const app = makeApp(); + patchConnectApp(app); + + const req = makeRequest(); + const res = makeResponse(); + const handler = vi.fn((_req: unknown, _res: unknown, n: () => void) => n()); + + app.use('/users', handler as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + + expect(startInactiveSpanCalls).toHaveLength(1); + const spanOptions = startInactiveSpanCalls[0]!.options as Record; + expect(spanOptions['name']).toBe('/users'); + expect((spanOptions['attributes'] as Record)['connect.type']).toBe('request_handler'); + expect((spanOptions['attributes'] as Record)['connect.name']).toBe('/users'); + expect((spanOptions['attributes'] as Record)['sentry.op']).toBe('request_handler.connect'); + expect((spanOptions['attributes'] as Record)['http.route']).toBe('/users'); + }); + + it('calls onRouteResolved when a named route handler is matched', () => { + const app = makeApp(); + const routeResolved: string[] = []; + patchConnectApp(app, { onRouteResolved: r => routeResolved.push(r) }); + + const req = makeRequest(); + const res = makeResponse(); + const handler = vi.fn((_req: unknown, _res: unknown, next: () => void) => next()); + + app.use('/api', handler as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + + expect(routeResolved).toStrictEqual(['/api']); + }); + + it('does not call onRouteResolved for anonymous middleware', () => { + const app = makeApp(); + const routeResolved: string[] = []; + patchConnectApp(app, { onRouteResolved: r => routeResolved.push(r) }); + + const req = makeRequest(); + const res = makeResponse(); + const middleware = vi.fn((_req: unknown, _res: unknown, next: () => void) => next()); + + app.use(middleware as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + + expect(routeResolved).toStrictEqual([]); + }); + + it('ends span when next() is called', () => { + const app = makeApp(); + patchConnectApp(app); + + const req = makeRequest(); + const res = makeResponse(); + let capturedNext: (() => void) | undefined; + const middleware = vi.fn((_req: unknown, _res: unknown, next: () => void) => { + capturedNext = next; + }); + + app.use(middleware as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + + expect(activeSpans[0]!.ended).toBe(false); + capturedNext!(); + expect(activeSpans[0]!.ended).toBe(true); + }); + + it('ends span when response close event fires', () => { + const app = makeApp(); + patchConnectApp(app); + + const req = makeRequest(); + const res = makeResponse(); + const middleware = vi.fn((_req: unknown, _res: unknown, _next: unknown) => { + // intentionally does not call next + }); + + app.use(middleware as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + + expect(activeSpans[0]!.ended).toBe(false); + // Simulate response close + res.listeners['close']?.forEach(fn => fn()); + expect(activeSpans[0]!.ended).toBe(true); + }); + + it('does not end span twice if both next() and close fire', () => { + const app = makeApp(); + patchConnectApp(app); + + const req = makeRequest(); + const res = makeResponse(); + let capturedNext: (() => void) | undefined; + const middleware = vi.fn((_req: unknown, _res: unknown, next: () => void) => { + capturedNext = next; + }); + + app.use(middleware as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + + capturedNext!(); + res.listeners['close']?.forEach(fn => fn()); + + // span.end is idempotent in our patchedMiddleware + expect(activeSpans[0]!.ended).toBe(true); + }); + + it('skips span creation when there is no active parent span', () => { + mockParentSpan = null; + const app = makeApp(); + patchConnectApp(app); + + const req = makeRequest(); + const res = makeResponse(); + const middleware = vi.fn((_req: unknown, _res: unknown, next: () => void) => next()); + + app.use(middleware as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + + expect(startInactiveSpanCalls).toHaveLength(0); + expect(middleware).toHaveBeenCalledOnce(); + }); + + it('preserves middleware arity (length) so connect can detect error handlers', () => { + const app = makeApp(); + patchConnectApp(app); + + const errorMiddleware = vi.fn(); + Object.defineProperty(errorMiddleware, 'length', { value: 4 }); + + app.use(errorMiddleware as unknown as (...args: unknown[]) => ConnectApp); + + // Verify that the patched middleware in the stack still has length 4 + const { stack } = app as unknown as { stack: Function[] }; + expect(stack[0]!.length).toBe(4); + }); + + it('error middleware uses (err, req, res, next) argument positions', () => { + const app = makeApp(); + patchConnectApp(app); + + const req = makeRequest(); + const res = makeResponse(); + const next = vi.fn(); + const err = new Error('oops'); + let capturedArgs: unknown[] = []; + + // 4-arg error middleware + const errorMiddleware = function (_err: unknown, _req: unknown, _res: unknown, _next: () => void) { + capturedArgs = [...arguments]; + _next(); + }; + + app.use(errorMiddleware as unknown as (...args: unknown[]) => ConnectApp); + + // Simulate connect calling the error middleware + const { stack } = app as unknown as { stack: Function[] }; + (stack[0] as Function)(err, req, res, next); + + expect(capturedArgs[0]).toBe(err); + expect(capturedArgs[1]).toBe(req); + expect(capturedArgs[2]).toBe(res); + expect(typeof capturedArgs[3]).toBe('function'); // patched next + }); + + it('wraps handle to track route stack per request', () => { + const app = makeApp(); + const routeResolved: string[] = []; + patchConnectApp(app, { onRouteResolved: r => routeResolved.push(r) }); + + const req = makeRequest(); + const res = makeResponse(); + + // Simulate nested: handle adds a layer, route handler resolves the route + const handler = vi.fn((_req: unknown, _res: unknown, next: () => void) => next()); + app.use('/api/users', handler as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + + expect(routeResolved).toStrictEqual(['/api/users']); + }); + + it('emits debug error when patching use fails (already wrapped)', () => { + const app = makeApp(); + patchConnectApp(app); // first patch + patchConnectApp(app); // second patch — should log debug error + expect(debugErrors.some(([msg]) => (msg as string).includes('use'))).toBe(true); + }); + + it('emits debug error when patching handle fails (already wrapped)', () => { + const app = makeApp(); + patchConnectApp(app); // first patch + patchConnectApp(app); // second patch — should log debug error + expect(debugErrors.some(([msg]) => (msg as string).includes('handle'))).toBe(true); + }); + + it('http.route falls back to "/" for middleware without a routeName', () => { + const app = makeApp(); + patchConnectApp(app); + + const req = makeRequest(); + const res = makeResponse(); + const middleware = vi.fn((_req: unknown, _res: unknown, next: () => void) => next()); + + app.use(middleware as unknown as (...args: unknown[]) => ConnectApp); + app.handle(req, res, () => undefined); + + const attrs = (startInactiveSpanCalls[0]!.options as { attributes: Record }).attributes; + expect(attrs['http.route']).toBe('/'); + }); +}); + +describe('setupConnectErrorHandler', () => { + it('adds a 4-argument error middleware to the app', () => { + const added: unknown[] = []; + const app = { use: (fn: unknown) => added.push(fn) }; + setupConnectErrorHandler(app); + expect(added).toHaveLength(1); + const fn = added[0] as Function; + expect(fn.length).toBe(4); + }); + + it('captures exceptions via captureException', () => { + const added: unknown[] = []; + const app = { use: (fn: unknown) => added.push(fn) }; + setupConnectErrorHandler(app); + + const next = vi.fn(); + const err = new Error('test'); + (added[0] as Function)(err, {}, {}, next); + + expect(capturedExceptions).toStrictEqual([ + [ + err, + { + mechanism: { handled: false, type: 'auto.middleware.connect' }, + }, + ], + ]); + expect(next).toHaveBeenCalledExactlyOnceWith(err); + }); +}); diff --git a/packages/node/src/integrations/tracing/connect.ts b/packages/node/src/integrations/tracing/connect.ts index 3fd7d10b28fb..ef23e0af9b61 100644 --- a/packages/node/src/integrations/tracing/connect.ts +++ b/packages/node/src/integrations/tracing/connect.ts @@ -1,29 +1,67 @@ -import { ConnectInstrumentation } from '@opentelemetry/instrumentation-connect'; -import type { IntegrationFn, Span } from '@sentry/core'; +// Automatic instrumentation for Connect using our portable core integration +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; +import { context } from '@opentelemetry/api'; +import { getRPCMetadata, RPCType } from '@opentelemetry/core'; + +import type { ConnectIntegrationOptions, ConnectModule, IntegrationFn } from '@sentry/core'; import { - captureException, + patchConnectModule, + setupConnectErrorHandler as coreSetupConnectErrorHandler, + SDK_VERSION, defineIntegration, - getClient, - SEMANTIC_ATTRIBUTE_SENTRY_OP, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - spanToJSON, } from '@sentry/core'; import { ensureIsWrapped, generateInstrumentOnce } from '@sentry/node-core'; type ConnectApp = { - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // oxlint-disable-next-line no-explicit-any use: (middleware: any) => void; }; const INTEGRATION_NAME = 'Connect'; +const SUPPORTED_VERSIONS = ['>=3.0.0 <4']; + +export type ConnectInstrumentationConfig = InstrumentationConfig & Omit; + +export const instrumentConnect = generateInstrumentOnce( + INTEGRATION_NAME, + (options?: ConnectInstrumentationConfig) => new ConnectInstrumentation(options), +); -export const instrumentConnect = generateInstrumentOnce(INTEGRATION_NAME, () => new ConnectInstrumentation()); +export class ConnectInstrumentation extends InstrumentationBase { + public constructor(config: ConnectInstrumentationConfig = {}) { + super('sentry-connect', SDK_VERSION, config); + } + + public init(): InstrumentationNodeModuleDefinition { + let originalConnect: ConnectModule | undefined; -const _connectIntegration = (() => { + return new InstrumentationNodeModuleDefinition( + 'connect', + SUPPORTED_VERSIONS, + connect => { + originalConnect = connect as ConnectModule; + return patchConnectModule(connect as ConnectModule, { + onRouteResolved(route) { + const rpcMetadata = getRPCMetadata(context.active()); + if (route && rpcMetadata?.type === RPCType.HTTP) { + rpcMetadata.route = route; + } + }, + }); + }, + () => { + return originalConnect; + }, + ); + } +} + +const _connectIntegration = ((options?: ConnectInstrumentationConfig) => { return { name: INTEGRATION_NAME, setupOnce() { - instrumentConnect(); + instrumentConnect(options); }, }; }) satisfies IntegrationFn; @@ -46,17 +84,6 @@ const _connectIntegration = (() => { */ export const connectIntegration = defineIntegration(_connectIntegration); -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function connectErrorMiddleware(err: any, req: any, res: any, next: any): void { - captureException(err, { - mechanism: { - handled: false, - type: 'auto.middleware.connect', - }, - }); - next(err); -} - /** * Add a Connect middleware to capture errors to Sentry. * @@ -71,46 +98,12 @@ function connectErrorMiddleware(err: any, req: any, res: any, next: any): void { * * Sentry.setupConnectErrorHandler(app); * - * // Add you connect routes here + * // Add your connect routes here * * app.listen(3000); * ``` */ export const setupConnectErrorHandler = (app: ConnectApp): void => { - app.use(connectErrorMiddleware); - - // Sadly, ConnectInstrumentation has no requestHook, so we need to add the attributes here - // We register this hook in this method, because if we register it in the integration `setup`, - // it would always run even for users that are not even using connect - const client = getClient(); - if (client) { - client.on('spanStart', span => { - addConnectSpanAttributes(span); - }); - } - + coreSetupConnectErrorHandler(app); ensureIsWrapped(app.use, 'connect'); }; - -function addConnectSpanAttributes(span: Span): void { - const attributes = spanToJSON(span).data; - - // this is one of: middleware, request_handler - const type = attributes['connect.type']; - - // If this is already set, or we have no connect span, no need to process again... - if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) { - return; - } - - span.setAttributes({ - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.connect', - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${type}.connect`, - }); - - // Also update the name, we don't need the "middleware - " prefix - const name = attributes['connect.name']; - if (typeof name === 'string') { - span.updateName(name); - } -}