From 40367c3bdf7c3e1b90e8e931203b35ec73182a47 Mon Sep 17 00:00:00 2001 From: Sebastian Alex Date: Tue, 8 Oct 2024 11:36:15 +0200 Subject: [PATCH 1/4] sdk-core: add application and application.version validation on initialize --- packages/sdk-core/src/BacktraceCoreClient.ts | 49 +++++++++ .../sdk-core/tests/client/clientTests.spec.ts | 100 ++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/packages/sdk-core/src/BacktraceCoreClient.ts b/packages/sdk-core/src/BacktraceCoreClient.ts index e9ff9c14..8302fdad 100644 --- a/packages/sdk-core/src/BacktraceCoreClient.ts +++ b/packages/sdk-core/src/BacktraceCoreClient.ts @@ -218,6 +218,8 @@ export abstract class BacktraceCoreClient< } public initialize() { + this.validateAttributes(); + if (this.fileSystem && this.options.database?.createDatabaseDirectory) { if (!this.options.database.path) { throw new Error( @@ -418,4 +420,51 @@ export abstract class BacktraceCoreClient< private static destroy() { this._instance = undefined; } + + private validateAttributes() { + function isNotEmptyString(v: unknown) { + if (typeof v === 'string' && v) { + return; + } + + return 'must be defined and not an empty string'; + } + + const validators = { + application: isNotEmptyString, + 'application.version': isNotEmptyString, + } as const; + + function validate(attributes: Record) { + const errors: Record = {}; + for (const [attribute, validate] of Object.entries(validators)) { + const value = attributes[attribute]; + const error = validate(value); + if (error) { + errors[attribute] = error; + } + } + return errors; + } + + // Validate scoped attributes first. If they return no errors, there is no point + // in checking all attributes, which resolving of may be slower. + const scoped = this.attributeManager.get('scoped'); + const scopedErrors = validate(scoped.attributes); + if (!Object.keys(scopedErrors).length) { + return; + } + + const allAttributes = this.attributeManager.get(); + const allErrors = validate(allAttributes.attributes); + if (!Object.keys(allErrors).length) { + return; + } + + const attributeErrors = Object.entries(allErrors) + .map(([attribute, error]) => `${attribute}: ${error}`) + .join('\n'); + + throw new Error(`Following attributes are invalid:\n${attributeErrors}`); + } } diff --git a/packages/sdk-core/tests/client/clientTests.spec.ts b/packages/sdk-core/tests/client/clientTests.spec.ts index 019db8c0..bf178244 100644 --- a/packages/sdk-core/tests/client/clientTests.spec.ts +++ b/packages/sdk-core/tests/client/clientTests.spec.ts @@ -1,6 +1,12 @@ import { BacktraceReport, BacktraceStringAttachment } from '../../src/index.js'; +import { AttributeManager } from '../../src/modules/attribute/AttributeManager.js'; import { BacktraceTestClient } from '../mocks/BacktraceTestClient.js'; +import { testHttpClient } from '../mocks/testHttpClient.js'; describe('Client tests', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + describe('Send tests', () => { const client = BacktraceTestClient.buildFakeClient(); @@ -128,4 +134,98 @@ describe('Client tests', () => { ); }); }); + + describe('Validation tests', () => { + it('should throw on initialize when application and application.version attributes are missing', () => { + const instance = new BacktraceTestClient({}, testHttpClient); + expect(() => instance.initialize()).toThrow(/application: must be defined and not an empty string/); + expect(() => instance.initialize()).toThrow(/application.version: must be defined and not an empty string/); + }); + + it('should throw on initialize when application attribute is missing', () => { + const instance = new BacktraceTestClient({}, testHttpClient, [ + { + type: 'scoped', + get: () => ({ + 'application.version': '1.2.3', + }), + }, + ]); + expect(() => instance.initialize()).toThrow(/application: must be defined and not an empty string/); + }); + + it('should throw on initialize when application.version attribute is missing', () => { + const instance = new BacktraceTestClient({}, testHttpClient, [ + { + type: 'scoped', + get: () => ({ + application: 'my-app', + }), + }, + ]); + expect(() => instance.initialize()).toThrow(/application.version: must be defined and not an empty string/); + }); + + it('should not throw on initialize when application and application.version attributes are defined as scoped', () => { + const instance = new BacktraceTestClient({}, testHttpClient, [ + { + type: 'scoped', + get: () => ({ + application: 'my-app', + 'application.version': '1.2.3', + }), + }, + ]); + expect(() => instance.initialize()).not.toThrow(); + }); + + it('should not throw on initialize when application and application.version attributes are defined as dynamic', () => { + const instance = new BacktraceTestClient({}, testHttpClient, [ + { + type: 'dynamic', + get: () => ({ + application: 'my-app', + 'application.version': '1.2.3', + }), + }, + ]); + expect(() => instance.initialize()).not.toThrow(); + }); + + it('should only test scoped attributes and not all when application and application.version attributes are defined as scoped', () => { + const instance = new BacktraceTestClient({}, testHttpClient, [ + { + type: 'scoped', + get: () => ({ + application: 'my-app', + 'application.version': '1.2.3', + }), + }, + ]); + + const getAttributesSpy = jest.spyOn(AttributeManager.prototype, 'get'); + instance.initialize(); + + expect(getAttributesSpy).toHaveBeenCalledWith('scoped'); + expect(getAttributesSpy).not.toHaveBeenCalledWith(); + }); + + it('should test both scoped attributes and all when application and application.version attributes are defined as dynamic', () => { + const instance = new BacktraceTestClient({}, testHttpClient, [ + { + type: 'dynamic', + get: () => ({ + application: 'my-app', + 'application.version': '1.2.3', + }), + }, + ]); + + const getAttributesSpy = jest.spyOn(AttributeManager.prototype, 'get'); + instance.initialize(); + + expect(getAttributesSpy).toHaveBeenCalledWith('scoped'); + expect(getAttributesSpy).toHaveBeenCalledWith(); + }); + }); }); From 10c9c2ebe5b4003393e5eaad7c86a861ece43acc Mon Sep 17 00:00:00 2001 From: Sebastian Alex Date: Tue, 8 Oct 2024 11:36:43 +0200 Subject: [PATCH 2/4] node: remove error from ApplicationInformationAttributeProvider on missing attributes --- .../attributes/ApplicationInformationAttributeProvider.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/node/src/attributes/ApplicationInformationAttributeProvider.ts b/packages/node/src/attributes/ApplicationInformationAttributeProvider.ts index a8e94392..b931a236 100644 --- a/packages/node/src/attributes/ApplicationInformationAttributeProvider.ts +++ b/packages/node/src/attributes/ApplicationInformationAttributeProvider.ts @@ -36,12 +36,6 @@ export class ApplicationInformationAttributeProvider implements BacktraceAttribu this._applicationVersion = this._applicationVersion ?? (applicationData['version'] as string); } - if (!this._application || !this._applicationVersion) { - throw new Error( - 'Cannot find information about the package. Please define application and application.version attribute', - ); - } - return { package: applicationData, [this.APPLICATION_ATTRIBUTE]: this._application, From 284a6fd27342b1080589ace5602b732f1a0b6ff7 Mon Sep 17 00:00:00 2001 From: Sebastian Alex Date: Tue, 8 Oct 2024 12:57:04 +0200 Subject: [PATCH 3/4] node: fix testing throw in ApplicationInformationAttributeProvider --- .../applicationInformationAttributeProviderTests.spec.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/node/tests/attributes/applicationInformationAttributeProviderTests.spec.ts b/packages/node/tests/attributes/applicationInformationAttributeProviderTests.spec.ts index 351df4f5..d2563c42 100644 --- a/packages/node/tests/attributes/applicationInformationAttributeProviderTests.spec.ts +++ b/packages/node/tests/attributes/applicationInformationAttributeProviderTests.spec.ts @@ -26,12 +26,5 @@ describe('Application information attribute provider tests', () => { expect(attributes[provider.APPLICATION_VERSION_ATTRIBUTE]).toBe(expectedPackageJson.version); }); - - it('Should throw an error when the package.json information does not exist', () => { - const testedPackageDir = path.join('/foo', 'bar', 'baz', '123', 'foo', 'bar'); - const provider = new ApplicationInformationAttributeProvider([testedPackageDir], {}); - - expect(() => provider.get()).toThrow(Error); - }); }); }); From 81a438fe6454da1da5c0bc6450a1be5f9f06f73b Mon Sep 17 00:00:00 2001 From: Sebastian Alex Date: Tue, 8 Oct 2024 13:11:29 +0200 Subject: [PATCH 4/4] sdk-core: simplify validateAttributes --- packages/sdk-core/src/BacktraceCoreClient.ts | 39 ++++--------------- .../sdk-core/tests/client/clientTests.spec.ts | 13 +++++-- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/packages/sdk-core/src/BacktraceCoreClient.ts b/packages/sdk-core/src/BacktraceCoreClient.ts index 8302fdad..82ed9dd2 100644 --- a/packages/sdk-core/src/BacktraceCoreClient.ts +++ b/packages/sdk-core/src/BacktraceCoreClient.ts @@ -422,49 +422,26 @@ export abstract class BacktraceCoreClient< } private validateAttributes() { - function isNotEmptyString(v: unknown) { - if (typeof v === 'string' && v) { - return; + function validateApplicationAndVersion(attributes: Record) { + if (!attributes['application'] || !attributes['application.version']) { + return 'application and application.version attributes must be defined.'; } - - return 'must be defined and not an empty string'; - } - - const validators = { - application: isNotEmptyString, - 'application.version': isNotEmptyString, - } as const; - - function validate(attributes: Record) { - const errors: Record = {}; - for (const [attribute, validate] of Object.entries(validators)) { - const value = attributes[attribute]; - const error = validate(value); - if (error) { - errors[attribute] = error; - } - } - return errors; } // Validate scoped attributes first. If they return no errors, there is no point // in checking all attributes, which resolving of may be slower. const scoped = this.attributeManager.get('scoped'); - const scopedErrors = validate(scoped.attributes); - if (!Object.keys(scopedErrors).length) { + const scopedError = validateApplicationAndVersion(scoped.attributes); + if (!scopedError) { return; } const allAttributes = this.attributeManager.get(); - const allErrors = validate(allAttributes.attributes); - if (!Object.keys(allErrors).length) { + const error = validateApplicationAndVersion(allAttributes.attributes); + if (!error) { return; } - const attributeErrors = Object.entries(allErrors) - .map(([attribute, error]) => `${attribute}: ${error}`) - .join('\n'); - - throw new Error(`Following attributes are invalid:\n${attributeErrors}`); + throw new Error(error); } } diff --git a/packages/sdk-core/tests/client/clientTests.spec.ts b/packages/sdk-core/tests/client/clientTests.spec.ts index bf178244..3e6d3f65 100644 --- a/packages/sdk-core/tests/client/clientTests.spec.ts +++ b/packages/sdk-core/tests/client/clientTests.spec.ts @@ -138,8 +138,9 @@ describe('Client tests', () => { describe('Validation tests', () => { it('should throw on initialize when application and application.version attributes are missing', () => { const instance = new BacktraceTestClient({}, testHttpClient); - expect(() => instance.initialize()).toThrow(/application: must be defined and not an empty string/); - expect(() => instance.initialize()).toThrow(/application.version: must be defined and not an empty string/); + expect(() => instance.initialize()).toThrow( + 'application and application.version attributes must be defined.', + ); }); it('should throw on initialize when application attribute is missing', () => { @@ -151,7 +152,9 @@ describe('Client tests', () => { }), }, ]); - expect(() => instance.initialize()).toThrow(/application: must be defined and not an empty string/); + expect(() => instance.initialize()).toThrow( + 'application and application.version attributes must be defined.', + ); }); it('should throw on initialize when application.version attribute is missing', () => { @@ -163,7 +166,9 @@ describe('Client tests', () => { }), }, ]); - expect(() => instance.initialize()).toThrow(/application.version: must be defined and not an empty string/); + expect(() => instance.initialize()).toThrow( + 'application and application.version attributes must be defined.', + ); }); it('should not throw on initialize when application and application.version attributes are defined as scoped', () => {