From da5e4641089fe54606fc1ab3dacc5e2d3abc135f Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:44:21 +0200 Subject: [PATCH 1/2] fix(node): Use the request method for Hapi routes with multiple methods --- .../suites/tracing/hapi/instrument-static.mjs | 10 +++ .../suites/tracing/hapi/scenario.mjs | 20 ++++++ .../suites/tracing/hapi/test.ts | 72 +++++++++++++++++++ .../integrations/hapi/hapi-error-handler.ts | 2 +- .../src/integrations/hapi/hapi-types.ts | 3 +- .../src/integrations/hapi/hapi-utils.ts | 26 +++++-- .../test/integrations/hapi-utils.test.ts | 38 ++++++---- 7 files changed, 149 insertions(+), 22 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/hapi/instrument-static.mjs diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/instrument-static.mjs b/dev-packages/node-integration-tests/suites/tracing/hapi/instrument-static.mjs new file mode 100644 index 000000000000..d799a8900e59 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/instrument-static.mjs @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, + traceLifecycle: 'static', +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.mjs index d081af0a4295..33a3df241a66 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.mjs @@ -18,6 +18,18 @@ const run = async () => { }, }); + server.route({ + method: ['GET', 'POST'], + path: '/multi', + handler: () => 'Hello World!', + }); + + server.route({ + method: '*', + path: '/wildcard', + handler: () => 'Hello World!', + }); + server.route({ method: 'GET', path: '/error', @@ -34,6 +46,14 @@ const run = async () => { }, }); + server.route({ + method: '*', + path: '/wildcard-error', + handler: () => { + return new Error('Sentry Test Error'); + }, + }); + server.route({ method: 'GET', path: '/boom-error', diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts index 24a9d1b2adba..3edf21b75df0 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts @@ -1,3 +1,5 @@ +import { HTTP_REQUEST_METHOD, SENTRY_OP } from '@sentry/conventions/attributes'; +import { ROUTER } from '@sentry/conventions/op'; import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; @@ -19,7 +21,36 @@ describe('hapi auto-instrumentation', () => { }, }; + const MULTI_METHOD_REQUESTS = [ + { method: 'get', path: '/multi' }, + { method: 'post', path: '/multi' }, + { method: 'put', path: '/wildcard' }, + ] as const; + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('should record the request method for multi-method and wildcard routes.', async () => { + const runner = createRunner().unordered(); + for (const { method, path } of MULTI_METHOD_REQUESTS) { + const httpMethod = method.toUpperCase(); + runner.expect({ + span: container => { + expect(container.items.find(item => item.is_segment)?.name).toBe(`${httpMethod} ${path}`); + + const routerSpans = container.items.filter(item => item.attributes[SENTRY_OP]?.value === ROUTER); + expect(routerSpans).toHaveLength(1); + expect(routerSpans[0]?.name).toBe(path); + expect(routerSpans[0]?.attributes[HTTP_REQUEST_METHOD]).toEqual({ type: 'string', value: httpMethod }); + }, + }); + } + + const started = runner.start(); + for (const { method, path } of MULTI_METHOD_REQUESTS) { + await started.makeRequest(method, path); + } + await started.completed(); + }); + test('should auto-instrument `@hapi/hapi` package.', async () => { const runner = createRunner() .expect({ @@ -114,6 +145,20 @@ describe('hapi auto-instrumentation', () => { await runner.completed(); }); + test('should assign the request method to the transactionName of errors in wildcard routes.', async () => { + const runner = createRunner() + .expect({ + event: { + ...EXPECTED_ERROR_EVENT, + transaction: 'PUT /wildcard-error', + }, + }) + .ignore('span') + .start(); + runner.makeRequest('put', '/wildcard-error', { expectError: true }); + await runner.completed(); + }); + test('should handle returned Boom errors in routes.', async () => { const runner = createRunner() .unordered() @@ -143,6 +188,33 @@ describe('hapi auto-instrumentation', () => { }); }); + // Only static spans put the method into the router span name, which crashed for multi-method routes. + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument-static.mjs', (createRunner, test) => { + test('should name router spans after the request method for multi-method and wildcard routes.', async () => { + const runner = createRunner().unordered(); + for (const { method, path } of MULTI_METHOD_REQUESTS) { + const httpMethod = method.toUpperCase(); + const spanName = `${httpMethod} ${path}`; + runner.expect({ + transaction: transaction => { + expect(transaction.transaction).toBe(spanName); + + const routerSpans = transaction.spans?.filter(span => span.op === ROUTER) ?? []; + expect(routerSpans).toHaveLength(1); + expect(routerSpans[0]?.description).toBe(spanName); + expect(routerSpans[0]?.data[HTTP_REQUEST_METHOD]).toBe(httpMethod); + }, + }); + } + + const started = runner.start(); + for (const { method, path } of MULTI_METHOD_REQUESTS) { + await started.makeRequest(method, path); + } + await started.completed(); + }); + }); + // Regression test: a `setupHapiErrorHandler` call before `server.start()` installs the default // predicate. The integration's own auto-registration (with a custom `shouldHandleError`) fires later, // at `server.start()`, and must still win. The custom predicate drops "Dropped error" (which the diff --git a/packages/server-utils/src/integrations/hapi/hapi-error-handler.ts b/packages/server-utils/src/integrations/hapi/hapi-error-handler.ts index d79b35b4502a..b6dbf152fc46 100644 --- a/packages/server-utils/src/integrations/hapi/hapi-error-handler.ts +++ b/packages/server-utils/src/integrations/hapi/hapi-error-handler.ts @@ -73,7 +73,7 @@ export function attachHapiErrorHandler(server: HapiServer, shouldHandleError?: H if (getIsolationScope() !== getDefaultIsolationScope()) { const route = request.route; if (route?.path) { - getIsolationScope().setTransactionName(`${route.method.toUpperCase()} ${route.path}`); + getIsolationScope().setTransactionName(`${request.method.toUpperCase()} ${route.path}`); } } else { DEBUG_BUILD && diff --git a/packages/server-utils/src/integrations/hapi/hapi-types.ts b/packages/server-utils/src/integrations/hapi/hapi-types.ts index 0afb23cc1448..f201c041c61a 100644 --- a/packages/server-utils/src/integrations/hapi/hapi-types.ts +++ b/packages/server-utils/src/integrations/hapi/hapi-types.ts @@ -29,7 +29,7 @@ export interface ServerRouteOptions { export interface ServerRoute { path: string; - method: string; + method: string | string[]; handler?: LifecycleMethod | unknown; options?: ((server: unknown) => ServerRouteOptions) | ServerRouteOptions; [key: string]: unknown; @@ -96,6 +96,7 @@ export interface HapiResponse { /** The subset of a hapi request the error listener reads. */ export interface HapiRequest { + method: string; route: { path?: string; method: string }; response?: HapiResponse; [key: string]: unknown; diff --git a/packages/server-utils/src/integrations/hapi/hapi-utils.ts b/packages/server-utils/src/integrations/hapi/hapi-utils.ts index 5c7afa11a268..4b589fae7eb2 100644 --- a/packages/server-utils/src/integrations/hapi/hapi-utils.ts +++ b/packages/server-utils/src/integrations/hapi/hapi-utils.ts @@ -29,7 +29,6 @@ import type { ServerExtEventsObject, ServerExtEventsRequestObject, ServerRequestExtType, - ServerRoute, ServerRouteOptions, } from './hapi-types'; import { HTTP_REQUEST_METHOD, HTTP_ROUTE } from '@sentry/conventions/attributes'; @@ -108,10 +107,18 @@ const isPatchableExtMethod = ( }; /** Build the span name and attributes for a Hapi route. */ -export const getRouteMetadata = (route: ServerRoute, pluginName?: string): SpanMetadata => { +export const getRouteMetadata = ({ + path, + method, + pluginName, +}: { + path: string; + method?: string; + pluginName?: string; +}): SpanMetadata => { const attributes: SpanAttributes = { - [HTTP_ROUTE]: route.path, - [HTTP_REQUEST_METHOD]: route.method, + [HTTP_ROUTE]: path, + [HTTP_REQUEST_METHOD]: method, }; if (pluginName) { @@ -126,10 +133,11 @@ export const getRouteMetadata = (route: ServerRoute, pluginName?: string): SpanM // handler spans are named after their route alone, without the method prefix. const isStreamedSpan = !!client && hasSpanStreamingEnabled(client); const fallbackName = pluginName ? REQUEST_HANDLER_SPAN_NAME_FALLBACK : ROUTER_SPAN_NAME_FALLBACK; + const staticName = method ? `${method} ${path}` : path; return { attributes, - name: isStreamedSpan ? route.path || fallbackName : `${route.method.toUpperCase()} ${route.path}`, + name: isStreamedSpan ? path || fallbackName : staticName, }; }; @@ -185,6 +193,12 @@ function startMetadataSpan(metadata: SpanMetadata, original: () => unknown): unk ); } +// The route config can register several methods (or `'*'`), so read the method +// Hapi resolved for this request instead. +function getRequestMethod(request: unknown): string | undefined { + return isObjectLike(request) && typeof request.method === 'string' ? request.method.toUpperCase() : undefined; +} + /** * Patches each individual route handler method in order to create the span. It * does not create spans when there is no parent span. @@ -199,7 +213,7 @@ function wrapRouteHandler(route: PatchableServerRoute, pluginName?: string): Pat return oldHandler.call(this, ...params); } setHttpServerSpanRouteAttribute(route.path); - const metadata = getRouteMetadata(route, pluginName); + const metadata = getRouteMetadata({ path: route.path, method: getRequestMethod(params[0]), pluginName }); return startMetadataSpan(metadata, () => oldHandler.call(this, ...params)); }; }; diff --git a/packages/server-utils/test/integrations/hapi-utils.test.ts b/packages/server-utils/test/integrations/hapi-utils.test.ts index a0a7a43971fa..6f9c06aa5047 100644 --- a/packages/server-utils/test/integrations/hapi-utils.test.ts +++ b/packages/server-utils/test/integrations/hapi-utils.test.ts @@ -1,10 +1,12 @@ +import { HTTP_REQUEST_METHOD, HTTP_ROUTE } from '@sentry/conventions/attributes'; import { setCurrentClient } from '@sentry/core'; import { afterEach, describe, expect, it } from 'vitest'; +import { AttributeNames } from '../../src/integrations/hapi/hapi-types'; import { getExtMetadata, getRouteMetadata } from '../../src/integrations/hapi/hapi-utils'; import { getDefaultTestClientOptions, TestClient } from '../mocks/client'; describe('getRouteMetadata', () => { - const route = { path: '/users/{id}', method: 'get' } as any; + const route = { path: '/users/{id}', method: 'GET' }; afterEach(() => { setCurrentClient(undefined as unknown as TestClient); @@ -14,21 +16,31 @@ describe('getRouteMetadata', () => { expect(getRouteMetadata(route)).toEqual({ name: 'GET /users/{id}', attributes: { - 'http.route': '/users/{id}', - 'http.request.method': 'get', - 'hapi.type': 'router', + [HTTP_ROUTE]: '/users/{id}', + [HTTP_REQUEST_METHOD]: 'GET', + [AttributeNames.HAPI_TYPE]: 'router', }, }); }); it('describes a plugin-registered route as a plugin layer', () => { - expect(getRouteMetadata(route, 'my-plugin')).toEqual({ + expect(getRouteMetadata({ ...route, pluginName: 'my-plugin' })).toEqual({ name: 'GET /users/{id}', attributes: { - 'http.route': '/users/{id}', - 'http.request.method': 'get', - 'hapi.type': 'plugin', - 'hapi.plugin.name': 'my-plugin', + [HTTP_ROUTE]: '/users/{id}', + [HTTP_REQUEST_METHOD]: 'GET', + [AttributeNames.HAPI_TYPE]: 'plugin', + [AttributeNames.PLUGIN_NAME]: 'my-plugin', + }, + }); + }); + + it('names the span after the path alone when the request method is unknown', () => { + expect(getRouteMetadata({ path: '/users/{id}' })).toEqual({ + name: '/users/{id}', + attributes: { + [HTTP_ROUTE]: '/users/{id}', + [AttributeNames.HAPI_TYPE]: 'router', }, }); }); @@ -44,17 +56,15 @@ describe('getRouteMetadata', () => { const client = new TestClient(getDefaultTestClientOptions({ traceLifecycle: 'stream' })); setCurrentClient(client); - expect(getRouteMetadata(route, 'my-plugin').name).toBe('/users/{id}'); + expect(getRouteMetadata({ ...route, pluginName: 'my-plugin' }).name).toBe('/users/{id}'); }); it('falls back to a static span name when the route has no path', () => { const client = new TestClient(getDefaultTestClientOptions({ traceLifecycle: 'stream' })); setCurrentClient(client); - const pathlessRoute = { path: '', method: 'get' } as any; - - expect(getRouteMetadata(pathlessRoute).name).toBe('Router'); - expect(getRouteMetadata(pathlessRoute, 'my-plugin').name).toBe('Request handler'); + expect(getRouteMetadata({ path: '', method: 'GET' }).name).toBe('Router'); + expect(getRouteMetadata({ path: '', method: 'GET', pluginName: 'my-plugin' }).name).toBe('Request handler'); }); }); From 173066d42500d415738a88e2eb3d3314c03367b1 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:10:56 +0200 Subject: [PATCH 2/2] test(node): Add Hapi route wrapper test for multi-method routes Co-authored-by: Rahul Kashyap Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- .../test/integrations/hapi-utils.test.ts | 44 +++++++++++++++++-- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a808dcc5947..ce959ba0ba08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott -Work in this release was contributed by @adityareddy-dev, @jpdefo, @Jxxunnn, @philipatkinson, @Shubham-Padkonde, @mikamikasuki, and @mketiku. Thank you for your contributions! +Work in this release was contributed by @adityareddy-dev, @jpdefo, @Jxxunnn, @philipatkinson, @Shubham-Padkonde, @mikamikasuki, @mketiku, and @itisrmk. Thank you for your contributions! ## 11.8.0 diff --git a/packages/server-utils/test/integrations/hapi-utils.test.ts b/packages/server-utils/test/integrations/hapi-utils.test.ts index 6f9c06aa5047..454e0e8f865a 100644 --- a/packages/server-utils/test/integrations/hapi-utils.test.ts +++ b/packages/server-utils/test/integrations/hapi-utils.test.ts @@ -1,8 +1,10 @@ -import { HTTP_REQUEST_METHOD, HTTP_ROUTE } from '@sentry/conventions/attributes'; -import { setCurrentClient } from '@sentry/core'; +import { HTTP_REQUEST_METHOD, HTTP_ROUTE, SENTRY_OP, SENTRY_ORIGIN } from '@sentry/conventions/attributes'; +import { ROUTER } from '@sentry/conventions/op'; +import type { Span } from '@sentry/core'; +import { setCurrentClient, spanToJSON, startSpan } from '@sentry/core'; import { afterEach, describe, expect, it } from 'vitest'; import { AttributeNames } from '../../src/integrations/hapi/hapi-types'; -import { getExtMetadata, getRouteMetadata } from '../../src/integrations/hapi/hapi-utils'; +import { getExtMetadata, getRouteMetadata, wrapRouteArguments } from '../../src/integrations/hapi/hapi-utils'; import { getDefaultTestClientOptions, TestClient } from '../mocks/client'; describe('getRouteMetadata', () => { @@ -68,6 +70,42 @@ describe('getRouteMetadata', () => { }); }); +describe('wrapRouteArguments', () => { + afterEach(() => { + setCurrentClient(undefined as unknown as TestClient); + }); + + it.each([ + { request: { method: 'post' }, name: 'POST /multi', method: 'POST' }, + { request: {}, name: '/multi', method: undefined }, + ])('names the router span $name for a multi-method route', ({ request, name, method }) => { + const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 })); + setCurrentClient(client); + client.init(); + const endedSpans: Span[] = []; + client.on('spanEnd', span => endedSpans.push(span)); + const route = { + method: ['GET', 'POST'], + path: '/multi', + handler: (_request: unknown, _h: unknown): unknown => 'ok', + }; + wrapRouteArguments([route]); + + const response = startSpan({ name: 'incoming' }, () => route.handler(request, {})); + + expect(response).toBe('ok'); + const routerSpan = endedSpans.map(span => spanToJSON(span)).find(span => span.attributes[SENTRY_OP] === ROUTER); + expect(routerSpan?.name).toBe(name); + expect(routerSpan?.attributes).toEqual({ + [HTTP_ROUTE]: '/multi', + [HTTP_REQUEST_METHOD]: method, + [AttributeNames.HAPI_TYPE]: 'router', + [SENTRY_OP]: ROUTER, + [SENTRY_ORIGIN]: 'auto.http.hapi', + }); + }); +}); + describe('getExtMetadata', () => { it('names an extension by its point', () => { expect(getExtMetadata('onPreHandler')).toEqual({