Skip to content

Commit b432b1d

Browse files
mydeamydea
committed
fix nestjs ???
1 parent 152143f commit b432b1d

File tree

3 files changed

+80
-49
lines changed

3 files changed

+80
-49
lines changed

‎CHANGELOG.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ This version updates the handling of the Node SDK of incoming requests. Instead
1313
This change should not affect users, unless they are relying on very in-depth implementation details. Importantly, this also drops the `_experimentalConfig` option of the integration - this will no longer do anything.
1414
Finally, you can still pass `instrumentation.{requestHook,responseHook,applyCustomAttributesOnSpan}` options, but they are deprecated and will be removed in v11. Instead, you can use the new `incomingRequestSpanHook` configuration option if you want to adjust the incoming request span.
1515

16-
1716
## 10.4.0
1817

1918
### Important Changes

‎packages/nestjs/src/integrations/sentry-nest-event-instrumentation.ts‎

Lines changed: 40 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
InstrumentationNodeModuleFile,
66
isWrapped,
77
} from '@opentelemetry/instrumentation';
8-
import { captureException, SDK_VERSION, startSpan } from '@sentry/core';
8+
import { captureException, handleCallbackErrors, SDK_VERSION, startSpan } from '@sentry/core';
99
import { getEventSpanOptions } from './helpers';
1010
import type { OnEventTarget } from './types';
1111

@@ -64,70 +64,68 @@ export class SentryNestEventInstrumentation extends InstrumentationBase {
6464

6565
// Return a new decorator function that wraps the handler
6666
return (target: OnEventTarget, propertyKey: string | symbol, descriptor: PropertyDescriptor) => {
67+
const originalHandler = descriptor.value;
68+
6769
if (
68-
!descriptor.value ||
69-
typeof descriptor.value !== 'function' ||
70+
!descriptorValueIsFunction(originalHandler) ||
7071
target.__SENTRY_INTERNAL__ ||
7172
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
7273
descriptor.value.__SENTRY_INSTRUMENTED__
7374
) {
7475
return decoratorResult(target, propertyKey, descriptor);
7576
}
7677

77-
const originalHandler = descriptor.value;
78-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
79-
const handlerName = originalHandler.name || propertyKey;
8078
let eventName = typeof event === 'string' ? event : String(event);
8179

8280
// Instrument the actual handler
83-
descriptor.value = async function (...args: unknown[]) {
84-
// When multiple @OnEvent decorators are used on a single method, we need to get all event names
85-
// from the reflector metadata as there is no information during execution which event triggered it
86-
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
87-
// @ts-ignore - reflect-metadata of nestjs adds these methods to Reflect
88-
if (Reflect.getMetadataKeys(descriptor.value).includes('EVENT_LISTENER_METADATA')) {
81+
descriptor.value = new Proxy(originalHandler, {
82+
apply: async function (target, thisArg, args) {
83+
// When multiple @OnEvent decorators are used on a single method, we need to get all event names
84+
// from the reflector metadata as there is no information during execution which event triggered it
8985
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
9086
// @ts-ignore - reflect-metadata of nestjs adds these methods to Reflect
91-
const eventData = Reflect.getMetadata('EVENT_LISTENER_METADATA', descriptor.value);
92-
if (Array.isArray(eventData)) {
93-
eventName = eventData
94-
.map((data: unknown) => {
95-
if (data && typeof data === 'object' && 'event' in data && data.event) {
96-
return data.event;
97-
}
98-
return '';
99-
})
100-
.reverse() // decorators are evaluated bottom to top
101-
.join('|');
87+
if (Reflect.getMetadataKeys(descriptor.value).includes('EVENT_LISTENER_METADATA')) {
88+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
89+
// @ts-ignore - reflect-metadata of nestjs adds these methods to Reflect
90+
const eventData = Reflect.getMetadata('EVENT_LISTENER_METADATA', descriptor.value);
91+
if (Array.isArray(eventData)) {
92+
eventName = eventData
93+
.map((data: unknown) => {
94+
if (data && typeof data === 'object' && 'event' in data && data.event) {
95+
return data.event;
96+
}
97+
return '';
98+
})
99+
.reverse() // decorators are evaluated bottom to top
100+
.join('|');
101+
}
102102
}
103-
}
104103

105-
return startSpan(getEventSpanOptions(eventName), async () => {
106-
try {
107-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
108-
const result = await originalHandler.apply(this, args);
109-
return result;
110-
} catch (error) {
111-
// exceptions from event handlers are not caught by global error filter
112-
captureException(error);
113-
throw error;
114-
}
115-
});
116-
};
104+
return startSpan(getEventSpanOptions(eventName), () => {
105+
return handleCallbackErrors(
106+
() => target.apply(thisArg, args),
107+
error => {
108+
captureException(error);
109+
},
110+
);
111+
});
112+
},
113+
});
117114

118115
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
119116
descriptor.value.__SENTRY_INSTRUMENTED__ = true;
120117

121-
// Preserve the original function name
122-
Object.defineProperty(descriptor.value, 'name', {
123-
value: handlerName,
124-
configurable: true,
125-
});
126-
127118
// Apply the original decorator
128119
return decoratorResult(target, propertyKey, descriptor);
129120
};
130121
};
131122
};
132123
}
133124
}
125+
126+
function descriptorValueIsFunction(
127+
value: unknown,
128+
// eslint-disable-next-line @typescript-eslint/ban-types
129+
): value is Function & { __SENTRY_INSTRUMENTED__?: boolean; name?: string } {
130+
return !!value && typeof value === 'function';
131+
}

‎packages/node-core/src/integrations/http/incoming-requests.ts‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* eslint-disable max-lines */
22
import type { Span } from '@opentelemetry/api';
3-
import { context, propagation, SpanKind, trace } from '@opentelemetry/api';
3+
import { context, createContextKey , propagation, SpanKind, trace } from '@opentelemetry/api';
44
import type { RPCMetadata } from '@opentelemetry/core';
55
import { getRPCMetadata, isTracingSuppressed, RPCType, setRPCMetadata } from '@opentelemetry/core';
66
import {
@@ -32,10 +32,18 @@ import type EventEmitter from 'events';
3232
import { errorMonitor } from 'events';
3333
import type { ClientRequest, IncomingHttpHeaders, IncomingMessage, Server, ServerResponse } from 'http';
3434
import type { Socket } from 'net';
35+
import { isProxy } from 'util/types';
3536
import { DEBUG_BUILD } from '../../debug-build';
3637
import type { NodeClient } from '../../sdk/client';
3738
import { INSTRUMENTATION_NAME, MAX_BODY_BYTE_LENGTH } from './constants';
3839

40+
type Emit = typeof Server.prototype.emit & {
41+
__sentry_patched__?: boolean;
42+
__sentryOriginalFn__?: typeof Server.prototype.emit;
43+
};
44+
45+
const HTTP_SERVER_INSTRUMENTED_KEY = createContextKey('sentry_http_server_instrumented');
46+
3947
const clientToRequestSessionAggregatesMap = new Map<
4048
Client,
4149
{ [timestampRoundedToSeconds: string]: { exited: number; crashed: number; errored: number } }
@@ -79,11 +87,14 @@ export function instrumentServer(
7987
};
8088
},
8189
): void {
90+
type Emit = typeof server.emit & { __sentryOriginalFn__?: typeof server.emit };
91+
8292
// eslint-disable-next-line @typescript-eslint/unbound-method
83-
const originalEmit = server.emit;
93+
const originalEmit: Emit = server.emit;
8494

85-
// This means it was already patched, do nothing
86-
if ((originalEmit as { __sentry_patched__?: boolean }).__sentry_patched__) {
95+
if (isEmitInstrumented(originalEmit)) {
96+
DEBUG_BUILD &&
97+
debug.log(INSTRUMENTATION_NAME, 'Incoming requests already instrumented, not instrumenting again...');
8798
return;
8899
}
89100

@@ -123,6 +134,11 @@ export function instrumentServer(
123134
return target.apply(thisArg, args);
124135
}
125136

137+
// Make sure we do not double wrap this, for edge cases...
138+
if (context.active().getValue(HTTP_SERVER_INSTRUMENTED_KEY)) {
139+
return target.apply(thisArg, args);
140+
}
141+
126142
DEBUG_BUILD && debug.log(INSTRUMENTATION_NAME, 'Handling incoming request');
127143

128144
const client = getClient<NodeClient>();
@@ -167,11 +183,14 @@ export function instrumentServer(
167183
// This way we can save an "unnecessary" `withScope()` invocation
168184
getCurrentScope().getPropagationContext().propagationSpanId = generateSpanId();
169185

170-
const ctx = propagation.extract(context.active(), normalizedRequest.headers);
186+
const ctx = propagation
187+
.extract(context.active(), normalizedRequest.headers)
188+
.setValue(HTTP_SERVER_INSTRUMENTED_KEY, true);
171189

172190
return context.with(ctx, () => {
173191
// if opting out of span creation, we can end here
174192
if (!spans || shouldIgnoreSpansForIncomingRequest(request) || !client) {
193+
DEBUG_BUILD && debug.log(INSTRUMENTATION_NAME, 'Skipping span creation for incoming request');
175194
return target.apply(thisArg, args);
176195
}
177196

@@ -263,7 +282,6 @@ export function instrumentServer(
263282
});
264283

265284
addNonEnumerableProperty(newEmit, '__sentry_patched__', true);
266-
267285
server.emit = newEmit;
268286
}
269287

@@ -552,3 +570,19 @@ export function isStaticAssetRequest(urlPath: string): boolean {
552570

553571
return false;
554572
}
573+
574+
function isEmitInstrumented(emit: Emit): boolean {
575+
// Easy: it does not have a __sentry_patched__ property? def. not instrumented
576+
if (!('__sentry_patched__' in emit)) {
577+
return false;
578+
}
579+
580+
// In weird cases with eventemitter2, it can _still_ be un-patched even if this propery is set :sad:
581+
// In this case, emit is not a proxy, which means it is not what we set it to
582+
// We'll wrap emit again in this case - but we also have code to ensure we do not double run our code inside of the wrapped emit
583+
if (!isProxy(emit)) {
584+
return false;
585+
}
586+
587+
return true;
588+
}

0 commit comments

Comments
 (0)