Skip to content

Commit

Permalink
fix(node/nestjs): Use method on current fastify request (#15066)
Browse files Browse the repository at this point in the history
The previous code was using fastify `request.routeOptions.method` which
is all methods the current route supports - not the current request
method.
Ex. `@All()` nestjs decorator `request.routeOptions.method = ['GET',
'POST', 'HEAD', ...]`

- Added a test for `@All()`
- Updated instances of fastify request to use `request.method` which
matches express

---------

Co-authored-by: Charly Gomez <[email protected]>
  • Loading branch information
tjhiggins and chargome authored Jan 21, 2025
1 parent 9a9fb89 commit 6bf2374
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Controller, Get, Param, ParseIntPipe, UseFilters, UseGuards, UseInterceptors } from '@nestjs/common';
import { All, Controller, Get, Param, ParseIntPipe, UseFilters, UseGuards, UseInterceptors } from '@nestjs/common';
import { flush } from '@sentry/nestjs';
import { AppService } from './app.service';
import { AsyncInterceptor } from './async-example.interceptor';
Expand Down Expand Up @@ -121,4 +121,9 @@ export class AppController {
testFunctionName() {
return this.appService.getFunctionName();
}

@All('test-all')
testAll() {
return {};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -808,3 +808,8 @@ test('Calling canActivate method on service with Injectable decorator returns 20
const response = await fetch(`${baseURL}/test-service-canActivate`);
expect(response.status).toBe(200);
});

test('Calling @All method on service with Injectable decorator returns 200', async ({ baseURL }) => {
const response = await fetch(`${baseURL}/test-all`);
expect(response.status).toBe(200);
});
2 changes: 1 addition & 1 deletion packages/nestjs/src/integrations/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
// https://github.com/fastify/fastify/blob/87f9f20687c938828f1138f91682d568d2a31e53/types/request.d.ts#L41
interface FastifyRequest {
routeOptions?: {
method?: string;
url?: string;
};
method?: string;
}

// Partial extract of ExpressRequest interface
Expand Down
6 changes: 2 additions & 4 deletions packages/nestjs/src/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ import { isExpectedError } from './helpers';
// https://github.com/fastify/fastify/blob/87f9f20687c938828f1138f91682d568d2a31e53/types/request.d.ts#L41
interface FastifyRequest {
routeOptions?: {
method?: string;
url?: string;
};
method?: string;
}

// Partial extract of ExpressRequest interface
Expand Down Expand Up @@ -57,9 +57,7 @@ class SentryTracingInterceptor implements NestInterceptor {
const req = context.switchToHttp().getRequest() as FastifyRequest | ExpressRequest;
if ('routeOptions' in req && req.routeOptions?.url) {
// fastify case
getIsolationScope().setTransactionName(
`${(req.routeOptions.method || 'GET').toUpperCase()} ${req.routeOptions.url}`,
);
getIsolationScope().setTransactionName(`${(req.method || 'GET').toUpperCase()} ${req.routeOptions.url}`);
} else if ('route' in req && req.route?.path) {
// express case
getIsolationScope().setTransactionName(`${(req.method || 'GET').toUpperCase()} ${req.route.path}`);
Expand Down
4 changes: 2 additions & 2 deletions packages/node/src/integrations/tracing/fastify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ interface Fastify {
* Works for Fastify 3, 4 and presumably 5.
*/
interface FastifyRequestRouteInfo {
method?: string;
// since [email protected]
routeOptions?: {
url?: string;
method?: string;
};
routerPath?: string;
}
Expand Down Expand Up @@ -107,7 +107,7 @@ export function setupFastifyErrorHandler(fastify: Fastify): void {
// Taken from Otel Fastify instrumentation:
// https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts#L94-L96
const routeName = reqWithRouteInfo.routeOptions?.url || reqWithRouteInfo.routerPath;
const method = reqWithRouteInfo.routeOptions?.method || 'GET';
const method = reqWithRouteInfo.method || 'GET';

getIsolationScope().setTransactionName(`${method} ${routeName}`);
});
Expand Down

0 comments on commit 6bf2374

Please sign in to comment.