From 10faabd809d7edfcc47f65621684de6214c31d1e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 11:20:55 +0100 Subject: [PATCH 1/5] ref(node): Refactor node integrations to use processEvent --- packages/node/src/integrations/context.ts | 52 ++++++++++--------- .../node/src/integrations/contextlines.ts | 15 +++--- packages/node/src/integrations/modules.ts | 40 +++++++------- 3 files changed, 55 insertions(+), 52 deletions(-) diff --git a/packages/node/src/integrations/context.ts b/packages/node/src/integrations/context.ts index f7044c509265..ee565c8676e7 100644 --- a/packages/node/src/integrations/context.ts +++ b/packages/node/src/integrations/context.ts @@ -11,7 +11,6 @@ import type { CultureContext, DeviceContext, Event, - EventProcessor, Integration, OsContext, } from '@sentry/types'; @@ -60,20 +59,25 @@ export class Context implements Integration { }, ) {} - /** - * @inheritDoc - */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void): void { - addGlobalEventProcessor(event => this.addContext(event)); + /** @inheritDoc */ + public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + // noop } - /** Processes an event and adds context */ + /** @inheritDoc */ + public processEvent(event: Event): Promise { + return this.addContext(event); + } + + /** + * Processes an event and adds context. + */ public async addContext(event: Event): Promise { if (this._cachedContext === undefined) { this._cachedContext = this._getContexts(); } - const updatedContext = this._updateContext(await this._cachedContext); + const updatedContext = _updateContext(await this._cachedContext); event.contexts = { ...event.contexts, @@ -87,22 +91,6 @@ export class Context implements Integration { return event; } - /** - * Updates the context with dynamic values that can change - */ - private _updateContext(contexts: Contexts): Contexts { - // Only update properties if they exist - if (contexts?.app?.app_memory) { - contexts.app.app_memory = process.memoryUsage().rss; - } - - if (contexts?.device?.free_memory) { - contexts.device.free_memory = os.freemem(); - } - - return contexts; - } - /** * Gets the contexts for the current environment */ @@ -137,6 +125,22 @@ export class Context implements Integration { } } +/** + * Updates the context with dynamic values that can change + */ +function _updateContext(contexts: Contexts): Contexts { + // Only update properties if they exist + if (contexts?.app?.app_memory) { + contexts.app.app_memory = process.memoryUsage().rss; + } + + if (contexts?.device?.free_memory) { + contexts.device.free_memory = os.freemem(); + } + + return contexts; +} + /** * Returns the operating system context. * diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts index c48f35adfd8b..2cc9375a879a 100644 --- a/packages/node/src/integrations/contextlines.ts +++ b/packages/node/src/integrations/contextlines.ts @@ -55,14 +55,13 @@ export class ContextLines implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - addGlobalEventProcessor(event => { - const self = getCurrentHub().getIntegration(ContextLines); - if (!self) { - return event; - } - return this.addSourceContext(event); - }); + public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + // noop + } + + /** @inheritDoc */ + public processEvent(event: Event): Promise { + return this.addSourceContext(event); } /** Processes an event and adds context lines */ diff --git a/packages/node/src/integrations/modules.ts b/packages/node/src/integrations/modules.ts index 9a81d5808425..cc8ecf621bb9 100644 --- a/packages/node/src/integrations/modules.ts +++ b/packages/node/src/integrations/modules.ts @@ -1,6 +1,6 @@ import { existsSync, readFileSync } from 'fs'; import { dirname, join } from 'path'; -import type { EventProcessor, Hub, Integration } from '@sentry/types'; +import type { Event, EventProcessor, Hub, Integration } from '@sentry/types'; let moduleCache: { [key: string]: string }; @@ -65,6 +65,14 @@ function collectModules(): { return infos; } +/** Fetches the list of modules and the versions loaded by the entry file for your node.js app. */ +function _getModules(): { [key: string]: string } { + if (!moduleCache) { + moduleCache = collectModules(); + } + return moduleCache; +} + /** Add node modules / packages to the event */ export class Modules implements Integration { /** @@ -80,26 +88,18 @@ export class Modules implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - addGlobalEventProcessor(event => { - if (!getCurrentHub().getIntegration(Modules)) { - return event; - } - return { - ...event, - modules: { - ...event.modules, - ...this._getModules(), - }, - }; - }); + public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + // noop } - /** Fetches the list of modules and the versions loaded by the entry file for your node.js app. */ - private _getModules(): { [key: string]: string } { - if (!moduleCache) { - moduleCache = collectModules(); - } - return moduleCache; + /** @inheritdoc */ + public processEvent(event: Event): Event { + return { + ...event, + modules: { + ...event.modules, + ..._getModules(), + }, + }; } } From b4fe4bc0235b5268e6083ca9d552bfe1bbf69160 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 15:29:18 +0100 Subject: [PATCH 2/5] try revert?? --- packages/node/src/integrations/contextlines.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts index 2cc9375a879a..c48f35adfd8b 100644 --- a/packages/node/src/integrations/contextlines.ts +++ b/packages/node/src/integrations/contextlines.ts @@ -55,13 +55,14 @@ export class ContextLines implements Integration { /** * @inheritDoc */ - public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - // noop - } - - /** @inheritDoc */ - public processEvent(event: Event): Promise { - return this.addSourceContext(event); + public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + addGlobalEventProcessor(event => { + const self = getCurrentHub().getIntegration(ContextLines); + if (!self) { + return event; + } + return this.addSourceContext(event); + }); } /** Processes an event and adds context lines */ From fa13155cf60f563b2405414c95ba01a17556a3f7 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 15:47:24 +0100 Subject: [PATCH 3/5] Revert "try revert??" This reverts commit cd98b89ee57cf22bfb9e3b861c7f5f92c0bb090e. --- packages/node/src/integrations/contextlines.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts index c48f35adfd8b..2cc9375a879a 100644 --- a/packages/node/src/integrations/contextlines.ts +++ b/packages/node/src/integrations/contextlines.ts @@ -55,14 +55,13 @@ export class ContextLines implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - addGlobalEventProcessor(event => { - const self = getCurrentHub().getIntegration(ContextLines); - if (!self) { - return event; - } - return this.addSourceContext(event); - }); + public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + // noop + } + + /** @inheritDoc */ + public processEvent(event: Event): Promise { + return this.addSourceContext(event); } /** Processes an event and adds context lines */ From 697586b392d0774de87153ed5671639770f2dd80 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 15:47:52 +0100 Subject: [PATCH 4/5] try fix it?? --- packages/node/test/index.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 3bb90a710708..fffd79bd6472 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -186,13 +186,16 @@ describe('SentryNode', () => { dsn, integrations: [new ContextLines()], }); - getCurrentHub().bindClient(new NodeClient(options)); + const client = new NodeClient(options); + getCurrentHub().bindClient(client); getCurrentScope().setTag('test', '1'); try { throw new Error('test'); } catch (e) { captureException(e); } + + void client.flush(); }); test('capture a linked exception with pre/post context', done => { From e83426e1887f7d4894dc1f83e6a1d4e8bf03c6af Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 15:59:53 +0100 Subject: [PATCH 5/5] try fix 2 --- packages/node/test/index.test.ts | 36 +++++++++++++++++--------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index fffd79bd6472..7f9abbdc1072 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -165,24 +165,24 @@ describe('SentryNode', () => { } }); - test('capture an exception with pre/post context', done => { - expect.assertions(10); + test('capture an exception with pre/post context', async () => { + const beforeSend = jest.fn((event: Event) => { + expect(event.tags).toEqual({ test: '1' }); + expect(event.exception).not.toBeUndefined(); + expect(event.exception!.values![0]).not.toBeUndefined(); + expect(event.exception!.values![0].stacktrace!).not.toBeUndefined(); + expect(event.exception!.values![0].stacktrace!.frames![1]).not.toBeUndefined(); + expect(event.exception!.values![0].stacktrace!.frames![1].pre_context).not.toBeUndefined(); + expect(event.exception!.values![0].stacktrace!.frames![1].post_context).not.toBeUndefined(); + expect(event.exception!.values![0].type).toBe('Error'); + expect(event.exception!.values![0].value).toBe('test'); + expect(event.exception!.values![0].stacktrace).toBeTruthy(); + return null; + }); + const options = getDefaultNodeClientOptions({ stackParser: defaultStackParser, - beforeSend: (event: Event) => { - expect(event.tags).toEqual({ test: '1' }); - expect(event.exception).not.toBeUndefined(); - expect(event.exception!.values![0]).not.toBeUndefined(); - expect(event.exception!.values![0].stacktrace!).not.toBeUndefined(); - expect(event.exception!.values![0].stacktrace!.frames![1]).not.toBeUndefined(); - expect(event.exception!.values![0].stacktrace!.frames![1].pre_context).not.toBeUndefined(); - expect(event.exception!.values![0].stacktrace!.frames![1].post_context).not.toBeUndefined(); - expect(event.exception!.values![0].type).toBe('Error'); - expect(event.exception!.values![0].value).toBe('test'); - expect(event.exception!.values![0].stacktrace).toBeTruthy(); - done(); - return null; - }, + beforeSend, dsn, integrations: [new ContextLines()], }); @@ -195,7 +195,9 @@ describe('SentryNode', () => { captureException(e); } - void client.flush(); + await client.flush(); + + expect(beforeSend).toHaveBeenCalledTimes(1); }); test('capture a linked exception with pre/post context', done => {