Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 6bf2374

Browse filesBrowse files
tjhigginschargome
tjhiggins
and
chargome
authored
fix(node/nestjs): Use method on current fastify request (getsentry#15066)
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 <charly.gomez@sentry.io>
1 parent 9a9fb89 commit 6bf2374
Copy full SHA for 6bf2374

File tree

Expand file treeCollapse file tree

5 files changed

+16
-8
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+16
-8
lines changed
Open diff view settings
Collapse file

‎dev-packages/e2e-tests/test-applications/nestjs-fastify/src/app.controller.ts‎

Copy file name to clipboardExpand all lines: dev-packages/e2e-tests/test-applications/nestjs-fastify/src/app.controller.ts
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Controller, Get, Param, ParseIntPipe, UseFilters, UseGuards, UseInterceptors } from '@nestjs/common';
1+
import { All, Controller, Get, Param, ParseIntPipe, UseFilters, UseGuards, UseInterceptors } from '@nestjs/common';
22
import { flush } from '@sentry/nestjs';
33
import { AppService } from './app.service';
44
import { AsyncInterceptor } from './async-example.interceptor';
@@ -121,4 +121,9 @@ export class AppController {
121121
testFunctionName() {
122122
return this.appService.getFunctionName();
123123
}
124+
125+
@All('test-all')
126+
testAll() {
127+
return {};
128+
}
124129
}
Collapse file

‎dev-packages/e2e-tests/test-applications/nestjs-fastify/tests/transactions.test.ts‎

Copy file name to clipboardExpand all lines: dev-packages/e2e-tests/test-applications/nestjs-fastify/tests/transactions.test.ts
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,3 +808,8 @@ test('Calling canActivate method on service with Injectable decorator returns 20
808808
const response = await fetch(`${baseURL}/test-service-canActivate`);
809809
expect(response.status).toBe(200);
810810
});
811+
812+
test('Calling @All method on service with Injectable decorator returns 200', async ({ baseURL }) => {
813+
const response = await fetch(`${baseURL}/test-all`);
814+
expect(response.status).toBe(200);
815+
});
Collapse file

‎packages/nestjs/src/integrations/types.ts‎

Copy file name to clipboardExpand all lines: packages/nestjs/src/integrations/types.ts
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
// https://github.com/fastify/fastify/blob/87f9f20687c938828f1138f91682d568d2a31e53/types/request.d.ts#L41
55
interface FastifyRequest {
66
routeOptions?: {
7-
method?: string;
87
url?: string;
98
};
9+
method?: string;
1010
}
1111

1212
// Partial extract of ExpressRequest interface
Collapse file

‎packages/nestjs/src/setup.ts‎

Copy file name to clipboardExpand all lines: packages/nestjs/src/setup.ts
+2-4Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ import { isExpectedError } from './helpers';
1616
// https://github.com/fastify/fastify/blob/87f9f20687c938828f1138f91682d568d2a31e53/types/request.d.ts#L41
1717
interface FastifyRequest {
1818
routeOptions?: {
19-
method?: string;
2019
url?: string;
2120
};
21+
method?: string;
2222
}
2323

2424
// Partial extract of ExpressRequest interface
@@ -57,9 +57,7 @@ class SentryTracingInterceptor implements NestInterceptor {
5757
const req = context.switchToHttp().getRequest() as FastifyRequest | ExpressRequest;
5858
if ('routeOptions' in req && req.routeOptions?.url) {
5959
// fastify case
60-
getIsolationScope().setTransactionName(
61-
`${(req.routeOptions.method || 'GET').toUpperCase()} ${req.routeOptions.url}`,
62-
);
60+
getIsolationScope().setTransactionName(`${(req.method || 'GET').toUpperCase()} ${req.routeOptions.url}`);
6361
} else if ('route' in req && req.route?.path) {
6462
// express case
6563
getIsolationScope().setTransactionName(`${(req.method || 'GET').toUpperCase()} ${req.route.path}`);
Collapse file

‎packages/node/src/integrations/tracing/fastify.ts‎

Copy file name to clipboardExpand all lines: packages/node/src/integrations/tracing/fastify.ts
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ interface Fastify {
2525
* Works for Fastify 3, 4 and presumably 5.
2626
*/
2727
interface FastifyRequestRouteInfo {
28+
method?: string;
2829
// since fastify@4.10.0
2930
routeOptions?: {
3031
url?: string;
31-
method?: string;
3232
};
3333
routerPath?: string;
3434
}
@@ -107,7 +107,7 @@ export function setupFastifyErrorHandler(fastify: Fastify): void {
107107
// Taken from Otel Fastify instrumentation:
108108
// https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts#L94-L96
109109
const routeName = reqWithRouteInfo.routeOptions?.url || reqWithRouteInfo.routerPath;
110-
const method = reqWithRouteInfo.routeOptions?.method || 'GET';
110+
const method = reqWithRouteInfo.method || 'GET';
111111

112112
getIsolationScope().setTransactionName(`${method} ${routeName}`);
113113
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.