Skip to content

Commit 72c3488

Browse files
authored
ref(node): Refactor node integrations to use processEvent (#9018)
This refactors Node integrations to use `processEvent`. Missing is the LocalVariables integration, as that is more complicated and may need to be refactored in a different way to properly work.
1 parent 5bc9a38 commit 72c3488

File tree

4 files changed

+77
-69
lines changed

4 files changed

+77
-69
lines changed

packages/node/src/integrations/context.ts

+28-24
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import type {
1111
CultureContext,
1212
DeviceContext,
1313
Event,
14-
EventProcessor,
1514
Integration,
1615
OsContext,
1716
} from '@sentry/types';
@@ -60,20 +59,25 @@ export class Context implements Integration {
6059
},
6160
) {}
6261

63-
/**
64-
* @inheritDoc
65-
*/
66-
public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void): void {
67-
addGlobalEventProcessor(event => this.addContext(event));
62+
/** @inheritDoc */
63+
public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void {
64+
// noop
6865
}
6966

70-
/** Processes an event and adds context */
67+
/** @inheritDoc */
68+
public processEvent(event: Event): Promise<Event> {
69+
return this.addContext(event);
70+
}
71+
72+
/**
73+
* Processes an event and adds context.
74+
*/
7175
public async addContext(event: Event): Promise<Event> {
7276
if (this._cachedContext === undefined) {
7377
this._cachedContext = this._getContexts();
7478
}
7579

76-
const updatedContext = this._updateContext(await this._cachedContext);
80+
const updatedContext = _updateContext(await this._cachedContext);
7781

7882
event.contexts = {
7983
...event.contexts,
@@ -87,22 +91,6 @@ export class Context implements Integration {
8791
return event;
8892
}
8993

90-
/**
91-
* Updates the context with dynamic values that can change
92-
*/
93-
private _updateContext(contexts: Contexts): Contexts {
94-
// Only update properties if they exist
95-
if (contexts?.app?.app_memory) {
96-
contexts.app.app_memory = process.memoryUsage().rss;
97-
}
98-
99-
if (contexts?.device?.free_memory) {
100-
contexts.device.free_memory = os.freemem();
101-
}
102-
103-
return contexts;
104-
}
105-
10694
/**
10795
* Gets the contexts for the current environment
10896
*/
@@ -137,6 +125,22 @@ export class Context implements Integration {
137125
}
138126
}
139127

128+
/**
129+
* Updates the context with dynamic values that can change
130+
*/
131+
function _updateContext(contexts: Contexts): Contexts {
132+
// Only update properties if they exist
133+
if (contexts?.app?.app_memory) {
134+
contexts.app.app_memory = process.memoryUsage().rss;
135+
}
136+
137+
if (contexts?.device?.free_memory) {
138+
contexts.device.free_memory = os.freemem();
139+
}
140+
141+
return contexts;
142+
}
143+
140144
/**
141145
* Returns the operating system context.
142146
*

packages/node/src/integrations/contextlines.ts

+7-8
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,13 @@ export class ContextLines implements Integration {
5555
/**
5656
* @inheritDoc
5757
*/
58-
public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
59-
addGlobalEventProcessor(event => {
60-
const self = getCurrentHub().getIntegration(ContextLines);
61-
if (!self) {
62-
return event;
63-
}
64-
return this.addSourceContext(event);
65-
});
58+
public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
59+
// noop
60+
}
61+
62+
/** @inheritDoc */
63+
public processEvent(event: Event): Promise<Event> {
64+
return this.addSourceContext(event);
6665
}
6766

6867
/** Processes an event and adds context lines */

packages/node/src/integrations/modules.ts

+20-20
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { existsSync, readFileSync } from 'fs';
22
import { dirname, join } from 'path';
3-
import type { EventProcessor, Hub, Integration } from '@sentry/types';
3+
import type { Event, EventProcessor, Hub, Integration } from '@sentry/types';
44

55
let moduleCache: { [key: string]: string };
66

@@ -65,6 +65,14 @@ function collectModules(): {
6565
return infos;
6666
}
6767

68+
/** Fetches the list of modules and the versions loaded by the entry file for your node.js app. */
69+
function _getModules(): { [key: string]: string } {
70+
if (!moduleCache) {
71+
moduleCache = collectModules();
72+
}
73+
return moduleCache;
74+
}
75+
6876
/** Add node modules / packages to the event */
6977
export class Modules implements Integration {
7078
/**
@@ -80,26 +88,18 @@ export class Modules implements Integration {
8088
/**
8189
* @inheritDoc
8290
*/
83-
public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
84-
addGlobalEventProcessor(event => {
85-
if (!getCurrentHub().getIntegration(Modules)) {
86-
return event;
87-
}
88-
return {
89-
...event,
90-
modules: {
91-
...event.modules,
92-
...this._getModules(),
93-
},
94-
};
95-
});
91+
public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
92+
// noop
9693
}
9794

98-
/** Fetches the list of modules and the versions loaded by the entry file for your node.js app. */
99-
private _getModules(): { [key: string]: string } {
100-
if (!moduleCache) {
101-
moduleCache = collectModules();
102-
}
103-
return moduleCache;
95+
/** @inheritdoc */
96+
public processEvent(event: Event): Event {
97+
return {
98+
...event,
99+
modules: {
100+
...event.modules,
101+
..._getModules(),
102+
},
103+
};
104104
}
105105
}

packages/node/test/index.test.ts

+22-17
Original file line numberDiff line numberDiff line change
@@ -165,34 +165,39 @@ describe('SentryNode', () => {
165165
}
166166
});
167167

168-
test('capture an exception with pre/post context', done => {
169-
expect.assertions(10);
168+
test('capture an exception with pre/post context', async () => {
169+
const beforeSend = jest.fn((event: Event) => {
170+
expect(event.tags).toEqual({ test: '1' });
171+
expect(event.exception).not.toBeUndefined();
172+
expect(event.exception!.values![0]).not.toBeUndefined();
173+
expect(event.exception!.values![0].stacktrace!).not.toBeUndefined();
174+
expect(event.exception!.values![0].stacktrace!.frames![1]).not.toBeUndefined();
175+
expect(event.exception!.values![0].stacktrace!.frames![1].pre_context).not.toBeUndefined();
176+
expect(event.exception!.values![0].stacktrace!.frames![1].post_context).not.toBeUndefined();
177+
expect(event.exception!.values![0].type).toBe('Error');
178+
expect(event.exception!.values![0].value).toBe('test');
179+
expect(event.exception!.values![0].stacktrace).toBeTruthy();
180+
return null;
181+
});
182+
170183
const options = getDefaultNodeClientOptions({
171184
stackParser: defaultStackParser,
172-
beforeSend: (event: Event) => {
173-
expect(event.tags).toEqual({ test: '1' });
174-
expect(event.exception).not.toBeUndefined();
175-
expect(event.exception!.values![0]).not.toBeUndefined();
176-
expect(event.exception!.values![0].stacktrace!).not.toBeUndefined();
177-
expect(event.exception!.values![0].stacktrace!.frames![1]).not.toBeUndefined();
178-
expect(event.exception!.values![0].stacktrace!.frames![1].pre_context).not.toBeUndefined();
179-
expect(event.exception!.values![0].stacktrace!.frames![1].post_context).not.toBeUndefined();
180-
expect(event.exception!.values![0].type).toBe('Error');
181-
expect(event.exception!.values![0].value).toBe('test');
182-
expect(event.exception!.values![0].stacktrace).toBeTruthy();
183-
done();
184-
return null;
185-
},
185+
beforeSend,
186186
dsn,
187187
integrations: [new ContextLines()],
188188
});
189-
getCurrentHub().bindClient(new NodeClient(options));
189+
const client = new NodeClient(options);
190+
getCurrentHub().bindClient(client);
190191
getCurrentScope().setTag('test', '1');
191192
try {
192193
throw new Error('test');
193194
} catch (e) {
194195
captureException(e);
195196
}
197+
198+
await client.flush();
199+
200+
expect(beforeSend).toHaveBeenCalledTimes(1);
196201
});
197202

198203
test('capture a linked exception with pre/post context', done => {

0 commit comments

Comments
 (0)