Skip to content

Commit d5fdd22

Browse files
authored
sdk-core, node: attribute validation (#296)
* sdk-core: add application and application.version validation on initialize * node: remove error from ApplicationInformationAttributeProvider on missing attributes * node: fix testing throw in ApplicationInformationAttributeProvider * sdk-core: simplify validateAttributes --------- Co-authored-by: Sebastian Alex <sebastian.alex@saucelabs.com>
1 parent e096dcf commit d5fdd22

File tree

4 files changed

+131
-13
lines changed

4 files changed

+131
-13
lines changed

packages/node/src/attributes/ApplicationInformationAttributeProvider.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,6 @@ export class ApplicationInformationAttributeProvider implements BacktraceAttribu
3636
this._applicationVersion = this._applicationVersion ?? (applicationData['version'] as string);
3737
}
3838

39-
if (!this._application || !this._applicationVersion) {
40-
throw new Error(
41-
'Cannot find information about the package. Please define application and application.version attribute',
42-
);
43-
}
44-
4539
return {
4640
package: applicationData,
4741
[this.APPLICATION_ATTRIBUTE]: this._application,

packages/node/tests/attributes/applicationInformationAttributeProviderTests.spec.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,5 @@ describe('Application information attribute provider tests', () => {
2626

2727
expect(attributes[provider.APPLICATION_VERSION_ATTRIBUTE]).toBe(expectedPackageJson.version);
2828
});
29-
30-
it('Should throw an error when the package.json information does not exist', () => {
31-
const testedPackageDir = path.join('/foo', 'bar', 'baz', '123', 'foo', 'bar');
32-
const provider = new ApplicationInformationAttributeProvider([testedPackageDir], {});
33-
34-
expect(() => provider.get()).toThrow(Error);
35-
});
3629
});
3730
});

packages/sdk-core/src/BacktraceCoreClient.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,8 @@ export abstract class BacktraceCoreClient<
218218
}
219219

220220
public initialize() {
221+
this.validateAttributes();
222+
221223
if (this.fileSystem && this.options.database?.createDatabaseDirectory) {
222224
if (!this.options.database.path) {
223225
throw new Error(
@@ -418,4 +420,28 @@ export abstract class BacktraceCoreClient<
418420
private static destroy() {
419421
this._instance = undefined;
420422
}
423+
424+
private validateAttributes() {
425+
function validateApplicationAndVersion(attributes: Record<string, unknown>) {
426+
if (!attributes['application'] || !attributes['application.version']) {
427+
return 'application and application.version attributes must be defined.';
428+
}
429+
}
430+
431+
// Validate scoped attributes first. If they return no errors, there is no point
432+
// in checking all attributes, which resolving of may be slower.
433+
const scoped = this.attributeManager.get('scoped');
434+
const scopedError = validateApplicationAndVersion(scoped.attributes);
435+
if (!scopedError) {
436+
return;
437+
}
438+
439+
const allAttributes = this.attributeManager.get();
440+
const error = validateApplicationAndVersion(allAttributes.attributes);
441+
if (!error) {
442+
return;
443+
}
444+
445+
throw new Error(error);
446+
}
421447
}

packages/sdk-core/tests/client/clientTests.spec.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { BacktraceReport, BacktraceStringAttachment } from '../../src/index.js';
2+
import { AttributeManager } from '../../src/modules/attribute/AttributeManager.js';
23
import { BacktraceTestClient } from '../mocks/BacktraceTestClient.js';
4+
import { testHttpClient } from '../mocks/testHttpClient.js';
35
describe('Client tests', () => {
6+
afterEach(() => {
7+
jest.restoreAllMocks();
8+
});
9+
410
describe('Send tests', () => {
511
const client = BacktraceTestClient.buildFakeClient();
612

@@ -128,4 +134,103 @@ describe('Client tests', () => {
128134
);
129135
});
130136
});
137+
138+
describe('Validation tests', () => {
139+
it('should throw on initialize when application and application.version attributes are missing', () => {
140+
const instance = new BacktraceTestClient({}, testHttpClient);
141+
expect(() => instance.initialize()).toThrow(
142+
'application and application.version attributes must be defined.',
143+
);
144+
});
145+
146+
it('should throw on initialize when application attribute is missing', () => {
147+
const instance = new BacktraceTestClient({}, testHttpClient, [
148+
{
149+
type: 'scoped',
150+
get: () => ({
151+
'application.version': '1.2.3',
152+
}),
153+
},
154+
]);
155+
expect(() => instance.initialize()).toThrow(
156+
'application and application.version attributes must be defined.',
157+
);
158+
});
159+
160+
it('should throw on initialize when application.version attribute is missing', () => {
161+
const instance = new BacktraceTestClient({}, testHttpClient, [
162+
{
163+
type: 'scoped',
164+
get: () => ({
165+
application: 'my-app',
166+
}),
167+
},
168+
]);
169+
expect(() => instance.initialize()).toThrow(
170+
'application and application.version attributes must be defined.',
171+
);
172+
});
173+
174+
it('should not throw on initialize when application and application.version attributes are defined as scoped', () => {
175+
const instance = new BacktraceTestClient({}, testHttpClient, [
176+
{
177+
type: 'scoped',
178+
get: () => ({
179+
application: 'my-app',
180+
'application.version': '1.2.3',
181+
}),
182+
},
183+
]);
184+
expect(() => instance.initialize()).not.toThrow();
185+
});
186+
187+
it('should not throw on initialize when application and application.version attributes are defined as dynamic', () => {
188+
const instance = new BacktraceTestClient({}, testHttpClient, [
189+
{
190+
type: 'dynamic',
191+
get: () => ({
192+
application: 'my-app',
193+
'application.version': '1.2.3',
194+
}),
195+
},
196+
]);
197+
expect(() => instance.initialize()).not.toThrow();
198+
});
199+
200+
it('should only test scoped attributes and not all when application and application.version attributes are defined as scoped', () => {
201+
const instance = new BacktraceTestClient({}, testHttpClient, [
202+
{
203+
type: 'scoped',
204+
get: () => ({
205+
application: 'my-app',
206+
'application.version': '1.2.3',
207+
}),
208+
},
209+
]);
210+
211+
const getAttributesSpy = jest.spyOn(AttributeManager.prototype, 'get');
212+
instance.initialize();
213+
214+
expect(getAttributesSpy).toHaveBeenCalledWith('scoped');
215+
expect(getAttributesSpy).not.toHaveBeenCalledWith();
216+
});
217+
218+
it('should test both scoped attributes and all when application and application.version attributes are defined as dynamic', () => {
219+
const instance = new BacktraceTestClient({}, testHttpClient, [
220+
{
221+
type: 'dynamic',
222+
get: () => ({
223+
application: 'my-app',
224+
'application.version': '1.2.3',
225+
}),
226+
},
227+
]);
228+
229+
const getAttributesSpy = jest.spyOn(AttributeManager.prototype, 'get');
230+
instance.initialize();
231+
232+
expect(getAttributesSpy).toHaveBeenCalledWith('scoped');
233+
expect(getAttributesSpy).toHaveBeenCalledWith();
234+
});
235+
});
131236
});

0 commit comments

Comments
 (0)