From 3fab403781d6e5de3f29d76d036d40c0fb56450d Mon Sep 17 00:00:00 2001 From: Konrad Dysput Date: Tue, 8 Aug 2023 13:16:07 +0200 Subject: [PATCH 1/4] Database support --- examples/sdk/node/.gitignore | 2 + examples/sdk/node/src/index.ts | 6 +- packages/node/src/BacktraceClient.ts | 3 + .../src/attachment/BacktraceFileAttachment.ts | 8 +- .../database/BacktraceDatabaseFileRecord.ts | 40 ++++ .../BacktraceDatabaseFileStorageProvider.ts | 98 +++++++++ packages/sdk-core/src/BacktraceCoreClient.ts | 55 ++++- packages/sdk-core/src/index.ts | 2 + .../BacktraceDatabaseConfiguration.ts | 9 +- .../src/modules/database/BacktraceDatabase.ts | 201 ++++++++++++++++++ .../database/BacktraceDatabaseContext.ts | 95 +++++++++ .../BacktraceDatabaseStorageProvider.ts | 8 + .../modules/database/DeduplicationModel.ts | 32 +++ .../sdk-core/src/modules/database/index.ts | 2 + .../database/model/BacktraceDatabaseRecord.ts | 14 ++ .../tests/client/clientCallbacksTests.spec.ts | 3 + .../databaseContextMemoryStorageTests.spec.ts | 119 +++++++++++ .../databaseContextValidationTests.spec.ts | 64 ++++++ .../database/databaseRecordBatchTests.spec.ts | 83 ++++++++ .../tests/database/databaseSetupTests.spec.ts | 117 ++++++++++ .../database/databaseStorageFlowTests.spec.ts | 112 ++++++++++ .../tests/mocks/BacktraceTestClient.ts | 22 +- .../tests/mocks/testStorageProvider.ts | 8 + 23 files changed, 1079 insertions(+), 24 deletions(-) create mode 100644 examples/sdk/node/.gitignore create mode 100644 packages/node/src/database/BacktraceDatabaseFileRecord.ts create mode 100644 packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts create mode 100644 packages/sdk-core/src/modules/database/BacktraceDatabase.ts create mode 100644 packages/sdk-core/src/modules/database/BacktraceDatabaseContext.ts create mode 100644 packages/sdk-core/src/modules/database/BacktraceDatabaseStorageProvider.ts create mode 100644 packages/sdk-core/src/modules/database/DeduplicationModel.ts create mode 100644 packages/sdk-core/src/modules/database/index.ts create mode 100644 packages/sdk-core/src/modules/database/model/BacktraceDatabaseRecord.ts create mode 100644 packages/sdk-core/tests/database/databaseContextMemoryStorageTests.spec.ts create mode 100644 packages/sdk-core/tests/database/databaseContextValidationTests.spec.ts create mode 100644 packages/sdk-core/tests/database/databaseRecordBatchTests.spec.ts create mode 100644 packages/sdk-core/tests/database/databaseSetupTests.spec.ts create mode 100644 packages/sdk-core/tests/database/databaseStorageFlowTests.spec.ts create mode 100644 packages/sdk-core/tests/mocks/testStorageProvider.ts diff --git a/examples/sdk/node/.gitignore b/examples/sdk/node/.gitignore new file mode 100644 index 00000000..b7fd1db9 --- /dev/null +++ b/examples/sdk/node/.gitignore @@ -0,0 +1,2 @@ +./database +./src/consts.ts \ No newline at end of file diff --git a/examples/sdk/node/src/index.ts b/examples/sdk/node/src/index.ts index 01cf3888..76ce96d9 100644 --- a/examples/sdk/node/src/index.ts +++ b/examples/sdk/node/src/index.ts @@ -11,7 +11,7 @@ const reader = readline.createInterface({ const client = BacktraceClient.builder({ url: SUBMISSION_URL, - attachments: [path.join(path.dirname(process.cwd()), 'samplefile.txt')], + attachments: [path.join(process.cwd(), 'samplefile.txt')], rateLimit: 5, userAttributes: { 'custom-attribute': 'test', @@ -20,6 +20,10 @@ const client = BacktraceClient.builder({ prop2: 123, }, }, + database: { + enabled: true, + path: path.join(process.cwd(), 'database'), + }, }).build(); console.log('Welcome to the @Backtrace/node demo'); diff --git a/packages/node/src/BacktraceClient.ts b/packages/node/src/BacktraceClient.ts index ba089d1c..7228ec5f 100644 --- a/packages/node/src/BacktraceClient.ts +++ b/packages/node/src/BacktraceClient.ts @@ -11,6 +11,7 @@ import { AGENT } from './agentDefinition'; import { BacktraceConfiguration } from './BacktraceConfiguration'; import { BacktraceClientBuilder } from './builder/BacktraceClientBuilder'; import { NodeOptionReader } from './common/NodeOptionReader'; +import { BacktraceDatabaseFileStorageProvider } from './database/BacktraceDatabaseFileStorageProvider'; export class BacktraceClient extends BacktraceCoreClient { constructor( @@ -26,6 +27,8 @@ export class BacktraceClient extends BacktraceCoreClient { undefined, undefined, new VariableDebugIdMapProvider(global as DebugIdContainer), + undefined, + BacktraceDatabaseFileStorageProvider.createIfValid(options.database), ); this.captureUnhandledErrors(options.captureUnhandledErrors, options.captureUnhandledPromiseRejections); diff --git a/packages/node/src/attachment/BacktraceFileAttachment.ts b/packages/node/src/attachment/BacktraceFileAttachment.ts index 1117fd02..42ba5246 100644 --- a/packages/node/src/attachment/BacktraceFileAttachment.ts +++ b/packages/node/src/attachment/BacktraceFileAttachment.ts @@ -5,14 +5,14 @@ import { Readable } from 'stream'; export class BacktraceFileAttachment implements BacktraceAttachment { public readonly name: string; - constructor(private readonly _filePath: string) { - this.name = path.basename(this._filePath); + constructor(public readonly filePath: string) { + this.name = path.basename(this.filePath); } public get(): fs.ReadStream | undefined { - if (!fs.existsSync(this._filePath)) { + if (!fs.existsSync(this.filePath)) { return undefined; } - return fs.createReadStream(this._filePath); + return fs.createReadStream(this.filePath); } } diff --git a/packages/node/src/database/BacktraceDatabaseFileRecord.ts b/packages/node/src/database/BacktraceDatabaseFileRecord.ts new file mode 100644 index 00000000..3bc50ec6 --- /dev/null +++ b/packages/node/src/database/BacktraceDatabaseFileRecord.ts @@ -0,0 +1,40 @@ +import { BacktraceData, BacktraceDatabaseRecord } from '@backtrace/sdk-core'; +import { BacktraceFileAttachment } from '../attachment'; + +export class BacktraceDatabaseFileRecord implements BacktraceDatabaseRecord { + public readonly data: BacktraceData; + public readonly id: string; + public readonly count: number; + public readonly hash: string; + public locked: boolean; + + private constructor(record: BacktraceDatabaseRecord, public readonly attachments: BacktraceFileAttachment[]) { + this.data = record.data; + this.id = record.id; + this.count = record.count; + this.hash = record.hash; + // make sure the database record stored in the database directory + // is never locked. By doing this, we want to be sure once we load + // the record once again, the record will be available for future usage + this.locked = false; + } + + public static fromRecord(record: BacktraceDatabaseRecord) { + return new BacktraceDatabaseFileRecord( + record, + record.attachments.filter((n) => n instanceof BacktraceFileAttachment) as BacktraceFileAttachment[], + ); + } + + public static fromJson(json: string): BacktraceDatabaseFileRecord | undefined { + try { + const record = JSON.parse(json) as BacktraceDatabaseFileRecord; + const attachments = record.attachments + ? record.attachments.map((n) => new BacktraceFileAttachment(n.filePath)) + : []; + return new BacktraceDatabaseFileRecord(record, attachments); + } catch { + return undefined; + } + } +} diff --git a/packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts b/packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts new file mode 100644 index 00000000..810e7583 --- /dev/null +++ b/packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts @@ -0,0 +1,98 @@ +import { + BacktraceDatabaseConfiguration, + BacktraceDatabaseRecord, + BacktraceDatabaseStorageProvider, +} from '@backtrace/sdk-core'; +import fs from 'fs'; +import * as fsPromise from 'fs/promises'; +import path from 'path'; +import { BacktraceDatabaseFileRecord } from './BacktraceDatabaseFileRecord'; +export class BacktraceDatabaseFileStorageProvider implements BacktraceDatabaseStorageProvider { + private _enabled = true; + + private readonly RECORD_SUFFIX = '-recod.json'; + private constructor(private readonly _path: string, private readonly _createDatabaseDirectory: boolean = false) {} + + /** + * Create a provider if provided options are valid + * @param options database configuration + * @returns database file storage provider + */ + public static createIfValid( + options?: BacktraceDatabaseConfiguration, + ): BacktraceDatabaseFileStorageProvider | undefined { + if (!options) { + return undefined; + } + if (!options.enabled || !options.path) { + return undefined; + } + + return new BacktraceDatabaseFileStorageProvider(options.path, options.createDatabaseDirectory); + } + + public start(): boolean { + // make sure by mistake we don't create anything or start any operation + if (this._enabled === false) { + return false; + } + + let databaseDirectoryExists = fs.existsSync(this._path); + if (this._createDatabaseDirectory !== false && !databaseDirectoryExists) { + fs.mkdirSync(this._path, { recursive: true }); + databaseDirectoryExists = true; + } + + return databaseDirectoryExists; + } + + public async delete(record: BacktraceDatabaseRecord): Promise { + const recordPath = this.getRecordPath(record.id); + if (!fs.existsSync(recordPath)) { + return false; + } + + await fsPromise.unlink(recordPath); + return true; + } + + public add(record: BacktraceDatabaseRecord): void { + const recordPath = this.getRecordPath(record.id); + fs.writeFileSync(recordPath, JSON.stringify(BacktraceDatabaseFileRecord.fromRecord(record)), { + encoding: 'utf8', + }); + } + + public async get(): Promise { + const databaseFiles = await fsPromise.readdir(this._path, { + encoding: 'utf8', + withFileTypes: true, + }); + + const recordNames = databaseFiles + .filter((file) => file.isFile() && file.name.endsWith(this.RECORD_SUFFIX)) + .map((n) => n.name); + + const records: BacktraceDatabaseRecord[] = []; + for (const recordName of recordNames) { + const recordPath = path.join(this._path, recordName); + try { + const recordJson = await fsPromise.readFile(recordPath, 'utf8'); + const record = BacktraceDatabaseFileRecord.fromJson(recordJson); + if (!record) { + await fsPromise.unlink(recordPath); + continue; + } + records.push(record); + } catch (err) { + await fsPromise.unlink(recordPath); + } + } + + return records; + } + + private getRecordPath(id: string): string { + return path.join(this._path, `${id}${this.RECORD_SUFFIX}`); + } +} diff --git a/packages/sdk-core/src/BacktraceCoreClient.ts b/packages/sdk-core/src/BacktraceCoreClient.ts index 5cc3623c..c4edb227 100644 --- a/packages/sdk-core/src/BacktraceCoreClient.ts +++ b/packages/sdk-core/src/BacktraceCoreClient.ts @@ -1,6 +1,8 @@ import { BacktraceAttachment, BacktraceAttributeProvider, + BacktraceDatabaseRecord, + BacktraceDatabaseStorageProvider, BacktraceSessionProvider, BacktraceStackTraceConverter, DebugIdMapProvider, @@ -18,6 +20,7 @@ import { BacktraceBreadcrumbs, BreadcrumbsSetup } from './modules/breadcrumbs'; import { BreadcrumbsManager } from './modules/breadcrumbs/BreadcrumbsManager'; import { V8StackTraceConverter } from './modules/converter/V8StackTraceConverter'; import { BacktraceDataBuilder } from './modules/data/BacktraceDataBuilder'; +import { BacktraceDatabase } from './modules/database/BacktraceDatabase'; import { BacktraceMetrics } from './modules/metrics/BacktraceMetrics'; import { MetricsBuilder } from './modules/metrics/MetricsBuilder'; import { SingleSessionProvider } from './modules/metrics/SingleSessionProvider'; @@ -65,6 +68,13 @@ export abstract class BacktraceCoreClient { return this.breadcrumbsManager; } + /** + * Report database used by the client + */ + public get database(): BacktraceDatabase | undefined { + return this._database; + } + /** * Client cached attachments */ @@ -76,6 +86,7 @@ export abstract class BacktraceCoreClient { private readonly _rateLimitWatcher: RateLimitWatcher; private readonly _attributeProvider: AttributeManager; private readonly _metrics?: BacktraceMetrics; + private readonly _database?: BacktraceDatabase; protected constructor( protected readonly options: BacktraceConfiguration, @@ -86,6 +97,7 @@ export abstract class BacktraceCoreClient { private readonly _sessionProvider: BacktraceSessionProvider = new SingleSessionProvider(), debugIdMapProvider?: DebugIdMapProvider, breadcrumbsSetup?: BreadcrumbsSetup, + databaseStorageProvider?: BacktraceDatabaseStorageProvider, ) { this._dataBuilder = new BacktraceDataBuilder( this._sdkOptions, @@ -94,7 +106,6 @@ export abstract class BacktraceCoreClient { ); this._reportSubmission = new BacktraceReportSubmission(options, requestHandler); - this._rateLimitWatcher = new RateLimitWatcher(options.rateLimit); this._attributeProvider = new AttributeManager([ new ClientAttributeProvider( _sdkOptions.agent, @@ -105,6 +116,18 @@ export abstract class BacktraceCoreClient { ...(attributeProviders ?? []), ]); this.attachments = options.attachments ?? []; + + if (databaseStorageProvider && options?.database?.enabled === true) { + this._database = new BacktraceDatabase( + this.options.database, + databaseStorageProvider, + this._reportSubmission, + ); + this._database.start(); + } + + this._rateLimitWatcher = new RateLimitWatcher(options.rateLimit); + const metrics = new MetricsBuilder(options, _sessionProvider, this._attributeProvider, requestHandler).build(); if (metrics) { this._metrics = metrics; @@ -179,7 +202,35 @@ export abstract class BacktraceCoreClient { return; } - await this._reportSubmission.send(backtraceData, this.generateSubmissionAttachments(report, reportAttachments)); + const submissionAttachments = this.generateSubmissionAttachments(report, reportAttachments); + const record = this.addToDatabase(backtraceData, submissionAttachments); + + const submissionResult = await this._reportSubmission.send(backtraceData, submissionAttachments); + if (!record) { + return; + } + record.locked = false; + if (submissionResult.status === 'Ok') { + await this._database?.remove(record); + } + } + + private addToDatabase( + data: BacktraceData, + attachments: BacktraceAttachment[], + ): BacktraceDatabaseRecord | undefined { + if (!this._database) { + return undefined; + } + + const record = this._database.add(data, attachments); + + if (!record || record.locked || record.count !== 1) { + return undefined; + } + + record.locked = true; + return record; } private generateSubmissionData(report: BacktraceReport): BacktraceData | undefined { diff --git a/packages/sdk-core/src/index.ts b/packages/sdk-core/src/index.ts index 88409b13..28f804f8 100644 --- a/packages/sdk-core/src/index.ts +++ b/packages/sdk-core/src/index.ts @@ -5,11 +5,13 @@ export * from './common/IdGenerator'; export * from './model/attachment'; export * from './model/configuration/BacktraceConfiguration'; export * from './model/configuration/BacktraceDatabaseConfiguration'; +export * from './model/data/BacktraceData'; export * from './model/http'; export * from './model/report/BacktraceErrorType'; export * from './model/report/BacktraceReport'; export * from './modules/attribute/BacktraceAttributeProvider'; export * from './modules/breadcrumbs'; export * from './modules/converter'; +export * from './modules/database'; export * from './modules/metrics/BacktraceSessionProvider'; export * from './sourcemaps/index'; diff --git a/packages/sdk-core/src/model/configuration/BacktraceDatabaseConfiguration.ts b/packages/sdk-core/src/model/configuration/BacktraceDatabaseConfiguration.ts index 3001832d..b43624dd 100644 --- a/packages/sdk-core/src/model/configuration/BacktraceDatabaseConfiguration.ts +++ b/packages/sdk-core/src/model/configuration/BacktraceDatabaseConfiguration.ts @@ -18,7 +18,7 @@ export enum DeduplicationStrategy { /** * Aggregates by faulting callstack, exception type, and exception message */ - All = ~(~0 << 4), + All = ~(~0 << 4) - 1, } export interface EnabledBacktraceDatabaseConfiguration { /** @@ -54,17 +54,12 @@ export interface EnabledBacktraceDatabaseConfiguration { */ maximumNumberOfRecords?: number; - /** - * The maximum database size in MB. When the limit is reached, the oldest reports are removed. - * If the value is equal to '0', then no limit is set. - * The default value is 0 (unlimited) - */ - maximumDatabaseSizeInMb?: number; /** * The amount of time (in ms) to wait between retries if the database is unable to send a report. * The default value is 60 000 */ retryInterval?: number; + /** * The maximum number of retries to attempt if the database is unable to send a report. * The default value is 3 diff --git a/packages/sdk-core/src/modules/database/BacktraceDatabase.ts b/packages/sdk-core/src/modules/database/BacktraceDatabase.ts new file mode 100644 index 00000000..2ab0446b --- /dev/null +++ b/packages/sdk-core/src/modules/database/BacktraceDatabase.ts @@ -0,0 +1,201 @@ +import { IdGenerator } from '../../common/IdGenerator'; +import { BacktraceAttachment } from '../../model/attachment'; +import { + BacktraceDatabaseConfiguration, + DeduplicationStrategy, +} from '../../model/configuration/BacktraceDatabaseConfiguration'; +import { BacktraceData } from '../../model/data/BacktraceData'; +import { BacktraceReportSubmission } from '../../model/http/BacktraceReportSubmission'; +import { BacktraceDatabaseContext } from './BacktraceDatabaseContext'; +import { BacktraceDatabaseStorageProvider } from './BacktraceDatabaseStorageProvider'; +import { DeduplicationModel } from './DeduplicationModel'; +import { BacktraceDatabaseRecord } from './model/BacktraceDatabaseRecord'; +export class BacktraceDatabase { + /** + * Determines if the database is enabled. + */ + public get enabled() { + return this._enabled; + } + + private readonly _databaseRecordContext: BacktraceDatabaseContext; + private readonly _deduplicationModel: DeduplicationModel; + + private readonly _maximumRecords: number; + private readonly _retryInterval: number; + private _intervalId?: ReturnType; + + private _enabled = false; + + constructor( + private readonly _options: BacktraceDatabaseConfiguration | undefined, + private readonly _storageProvider: BacktraceDatabaseStorageProvider, + private readonly _requestHandler: BacktraceReportSubmission, + ) { + this._databaseRecordContext = new BacktraceDatabaseContext(this._options?.maximumRetries); + this._deduplicationModel = new DeduplicationModel( + this._options?.deduplicationStrategy ?? DeduplicationStrategy.None, + ); + this._maximumRecords = this._options?.maximumNumberOfRecords ?? 8; + this._retryInterval = this._options?.retryInterval ?? 60_000; + } + + /** + * Starts database integration. + * @returns true if the database started successfully. Otherwise false. + */ + public start(): boolean { + if (this._enabled) { + return this._enabled; + } + + if (this._options?.enabled === false) { + return false; + } + + const startResult = this._storageProvider.start(); + if (!startResult) { + return false; + } + + this.loadReports().then(async () => { + await this.setupDatabaseAutoSend(); + }); + this._enabled = true; + return true; + } + + /** + * Adds backtrace data to the database + * @param backtraceData diagnostic data object + * @param attachments attachments + * @returns record if database is enabled. Otherwise undefined. + */ + public add( + backtraceData: BacktraceData, + attachments: BacktraceAttachment[], + ): BacktraceDatabaseRecord | undefined { + if (!this._enabled) { + return undefined; + } + + const dataHash = this._deduplicationModel.getSha(backtraceData); + if (dataHash) { + const existingRecord = this._databaseRecordContext.find((record) => record.hash === dataHash); + if (existingRecord) { + existingRecord.count++; + return existingRecord; + } + } + + this.prepareDatabase(); + + const record = { + count: 1, + data: backtraceData, + hash: dataHash, + id: IdGenerator.uuid(), + locked: false, + attachments: attachments, + }; + this._storageProvider.add(record); + this._databaseRecordContext.add(record); + + return record; + } + + /** + * Returns stored references to Backtrace records + * @returns available records in the database + */ + public get(): BacktraceDatabaseRecord[] { + return this._databaseRecordContext.get(); + } + + /** + * @returns Returns number of records stored in the Database + */ + public count(): number { + return this._databaseRecordContext.count(); + } + + /** + * Disables database integration. After running the dispose method, you cannot + * execute any database operations. + */ + public dispose() { + this._enabled = false; + clearInterval(this._intervalId); + } + + /** + * Removes the database record + * @param record database records + */ + public async remove(record: BacktraceDatabaseRecord) { + if (!this._enabled) { + return; + } + this._databaseRecordContext.remove(record); + await this._storageProvider.delete(record); + } + + /** + * Prepare database to insert records + * @param totalNumberOfRecords number of records to insert + */ + private prepareDatabase(totalNumberOfRecords = 1) { + const numberOfRecords = this.count(); + if (numberOfRecords + totalNumberOfRecords > this._maximumRecords) { + this._databaseRecordContext.dropOverflow(totalNumberOfRecords); + } + } + + private async loadReports(): Promise { + const records = await this._storageProvider.get(); + if (records.length > this._maximumRecords) { + records.length = this._maximumRecords; + } + this.prepareDatabase(records.length); + this._databaseRecordContext.load(records); + } + + private async setupDatabaseAutoSend() { + if (this._options?.autoSend === false) { + return; + } + + const sendDatabaseReports = async () => { + await this.sendRecords(); + }; + this._intervalId = setInterval(sendDatabaseReports, this._retryInterval); + await this.sendRecords(); + } + + private async sendRecords() { + let submissionFailure = false; + for (let bucketIndex = 0; bucketIndex < this._databaseRecordContext.bucketCount; bucketIndex++) { + if (submissionFailure) { + break; + } + for (const record of this._databaseRecordContext.getBucket(bucketIndex)) { + if (record.locked) { + continue; + } + try { + record.locked = true; + const result = await this._requestHandler.send(record.data, record.attachments); + if (result.status === 'Ok') { + this.remove(record); + continue; + } + submissionFailure = true; + this._databaseRecordContext.increaseBucket(bucketIndex); + break; + } finally { + record.locked = false; + } + } + } + } +} diff --git a/packages/sdk-core/src/modules/database/BacktraceDatabaseContext.ts b/packages/sdk-core/src/modules/database/BacktraceDatabaseContext.ts new file mode 100644 index 00000000..3a924981 --- /dev/null +++ b/packages/sdk-core/src/modules/database/BacktraceDatabaseContext.ts @@ -0,0 +1,95 @@ +import { BacktraceDatabaseRecord } from './model/BacktraceDatabaseRecord'; + +export class BacktraceDatabaseContext { + public readonly recordBucket: BacktraceDatabaseRecord[][]; + constructor(public readonly bucketCount: number = 3) { + this.recordBucket = this.setupRecordBucket(this.bucketCount); + } + + public find(predicate: (record: BacktraceDatabaseRecord) => boolean): BacktraceDatabaseRecord | undefined { + for (let index = 0; index < this.bucketCount; index++) { + for (const record of this.recordBucket[index]) { + if (predicate(record)) { + return record; + } + } + } + return undefined; + } + + public add(record: BacktraceDatabaseRecord): void { + this.recordBucket[0].push(record); + } + + public get(): BacktraceDatabaseRecord[] { + const result = []; + for (const bucket of this.recordBucket) { + result.push( + ...bucket.map((n) => { + return { ...n }; + }), + ); + } + + return result; + } + + public getBucket(index: number) { + return this.recordBucket[index]; + } + + public count() { + return Object.values(this.recordBucket) + .map((n) => n.reduce((total, record) => total + record.count, 0)) + .reduce((total, current) => total + current, 0); + } + + public remove(databaseRecord: BacktraceDatabaseRecord): void { + for (let bucketIndex = 0; bucketIndex < this.bucketCount; bucketIndex++) { + for (let recordIndex = 0; recordIndex < this.recordBucket[bucketIndex].length; recordIndex++) { + const record = this.recordBucket[bucketIndex][recordIndex]; + if (databaseRecord.id === record.id) { + this.recordBucket[bucketIndex].splice(recordIndex, 1); + return; + } + } + } + } + + public increaseBucket(bucketStart: number) { + for (let bucketIndex = this.bucketCount - 1; bucketIndex >= bucketStart; bucketIndex--) { + if (bucketIndex === this.bucketCount - 1) { + this.recordBucket[bucketIndex] = []; + continue; + } + + this.recordBucket[bucketIndex + 1] = this.recordBucket[bucketIndex]; + this.recordBucket[bucketIndex] = []; + } + } + + public load(records: BacktraceDatabaseRecord[]): void { + this.recordBucket[0].push(...records); + } + + public dropOverflow(overflow: number) { + for (let bucketIndex = this.bucketCount - 1; bucketIndex >= 0; bucketIndex--) { + const bucket = this.recordBucket[bucketIndex]; + const removedRecords = bucket.splice(0, overflow); + overflow = overflow - removedRecords.length; + + if (overflow === 0) { + return; + } + } + } + + private setupRecordBucket(retries: number): BacktraceDatabaseRecord[][] { + const result: BacktraceDatabaseRecord[][] = []; + for (let index = 0; index < retries; index++) { + result[index] = []; + } + + return result; + } +} diff --git a/packages/sdk-core/src/modules/database/BacktraceDatabaseStorageProvider.ts b/packages/sdk-core/src/modules/database/BacktraceDatabaseStorageProvider.ts new file mode 100644 index 00000000..2418b5d9 --- /dev/null +++ b/packages/sdk-core/src/modules/database/BacktraceDatabaseStorageProvider.ts @@ -0,0 +1,8 @@ +import { BacktraceDatabaseRecord } from './model/BacktraceDatabaseRecord'; + +export interface BacktraceDatabaseStorageProvider { + add(databaseRecord: BacktraceDatabaseRecord): void; + get(): Promise; + start(): boolean; + delete(record: BacktraceDatabaseRecord): Promise; +} diff --git a/packages/sdk-core/src/modules/database/DeduplicationModel.ts b/packages/sdk-core/src/modules/database/DeduplicationModel.ts new file mode 100644 index 00000000..65745c81 --- /dev/null +++ b/packages/sdk-core/src/modules/database/DeduplicationModel.ts @@ -0,0 +1,32 @@ +import { BacktraceData } from '@backtrace/sdk-core/src/model/data/BacktraceData'; +import crypto from 'crypto'; +import { DeduplicationStrategy } from '../..'; + +export class DeduplicationModel { + private readonly CUSTOM_FINGERPRINT_ATTRIBUTE = '_mod_fingerprint'; + constructor(private readonly _deduplicationStrategy: DeduplicationStrategy) {} + + public getSha(data: BacktraceData): string { + const customFingerprint = data.attributes[this.CUSTOM_FINGERPRINT_ATTRIBUTE] as string; + if (customFingerprint) { + return customFingerprint; + } + + if (this._deduplicationStrategy === DeduplicationStrategy.None) { + return ''; + } + + const deduplicationPayload = + ((this._deduplicationStrategy & DeduplicationStrategy.Classifier) == DeduplicationStrategy.Classifier + ? data.classifiers.join(',') + : '') + + ((this._deduplicationStrategy & DeduplicationStrategy.Callstack) == DeduplicationStrategy.Callstack + ? JSON.stringify(data.threads[data.mainThread]) + : '') + + ((this._deduplicationStrategy & DeduplicationStrategy.Message) === DeduplicationStrategy.Message + ? data.attributes['error.message'] + : ''); + + return crypto.createHash('sha256').update(deduplicationPayload).digest('hex'); + } +} diff --git a/packages/sdk-core/src/modules/database/index.ts b/packages/sdk-core/src/modules/database/index.ts new file mode 100644 index 00000000..448c4cd1 --- /dev/null +++ b/packages/sdk-core/src/modules/database/index.ts @@ -0,0 +1,2 @@ +export * from './BacktraceDatabaseStorageProvider'; +export * from './model/BacktraceDatabaseRecord'; diff --git a/packages/sdk-core/src/modules/database/model/BacktraceDatabaseRecord.ts b/packages/sdk-core/src/modules/database/model/BacktraceDatabaseRecord.ts new file mode 100644 index 00000000..4f330def --- /dev/null +++ b/packages/sdk-core/src/modules/database/model/BacktraceDatabaseRecord.ts @@ -0,0 +1,14 @@ +import { BacktraceAttachment } from '../../../model/attachment'; +import { BacktraceData } from '../../../model/data/BacktraceData'; + +export interface BacktraceDatabaseRecord { + data: BacktraceData; + id: string; + attachments: BacktraceAttachment[]; + count: number; + hash: string; + /** + * Determines if the record is in use + */ + locked: boolean; +} diff --git a/packages/sdk-core/tests/client/clientCallbacksTests.spec.ts b/packages/sdk-core/tests/client/clientCallbacksTests.spec.ts index 16853d70..a29aa4a6 100644 --- a/packages/sdk-core/tests/client/clientCallbacksTests.spec.ts +++ b/packages/sdk-core/tests/client/clientCallbacksTests.spec.ts @@ -3,6 +3,9 @@ import { BacktraceData } from '../../src/model/data/BacktraceData'; import { BacktraceTestClient } from '../mocks/BacktraceTestClient'; describe('Client callbacks tests', () => { + afterEach(() => { + jest.clearAllMocks(); + }); describe('Submission data modification tests', () => { it('Should invoke the before send event', async () => { let triggered = false; diff --git a/packages/sdk-core/tests/database/databaseContextMemoryStorageTests.spec.ts b/packages/sdk-core/tests/database/databaseContextMemoryStorageTests.spec.ts new file mode 100644 index 00000000..d82cb989 --- /dev/null +++ b/packages/sdk-core/tests/database/databaseContextMemoryStorageTests.spec.ts @@ -0,0 +1,119 @@ +import path from 'path'; +import { BacktraceData, BacktraceDatabaseRecord, BacktraceReportSubmissionResult } from '../../src'; +import { BacktraceDatabase } from '../../src/modules/database/BacktraceDatabase'; +import { BacktraceTestClient } from '../mocks/BacktraceTestClient'; +import { testStorageProvider } from '../mocks/testStorageProvider'; + +describe('Database context memory storage tests', () => { + const testDatabaseSettings = { + enabled: true, + autoSend: false, + // this option doesn't matter because we mock the database provider + // interface. However, if bug happen we want to be sure to not create + // anything. Instead we want to fail loud and hard. + createDatabaseDirectory: false, + path: path.join(__dirname, 'database'), + }; + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('Adding reports to the database via client API', () => { + it('Should add report to the database via client send method', async () => { + const testingErrorMessage = 'testingErrorMessage'; + const client = BacktraceTestClient.buildFakeClient( + { + database: testDatabaseSettings, + }, + [], + [], + testStorageProvider, + ); + const database = client.database as BacktraceDatabase; + if (!database) { + throw new Error('Invalid database setup. Database must be defined!'); + } + + jest.spyOn(client.requestHandler, 'postError').mockResolvedValue( + Promise.resolve(BacktraceReportSubmissionResult.OnInternalServerError('test')), + ); + + await client.send(new Error(testingErrorMessage)); + + const records = database.get(); + + expect(records.length).toBe(1); + expect(records[0].data.attributes['error.message']).toEqual(testingErrorMessage); + }); + + it('Should remove report from the database after succesful submission', async () => { + const testingErrorMessage = 'testingErrorMessage'; + const client = BacktraceTestClient.buildFakeClient( + { + database: testDatabaseSettings, + }, + [], + [], + testStorageProvider, + ); + const database = client.database as BacktraceDatabase; + if (!database) { + throw new Error('Invalid database setup. Database must be defined!'); + } + + jest.spyOn(client.requestHandler, 'postError').mockResolvedValue( + Promise.resolve(BacktraceReportSubmissionResult.Ok({})), + ); + + await client.send(new Error(testingErrorMessage)); + + const records = database.get(); + + expect(records.length).toBe(0); + expect(testStorageProvider.add).toHaveBeenCalled(); + }); + }); + + describe('Record load on the database start', () => { + it('Shouldn not fail when no records are available in the database dir', () => { + jest.spyOn(testStorageProvider, 'start').mockReturnValue(true); + jest.spyOn(testStorageProvider, 'get').mockResolvedValue(Promise.resolve([])); + const client = BacktraceTestClient.buildFakeClient( + { + database: testDatabaseSettings, + }, + [], + [], + testStorageProvider, + ); + + expect((client.database as BacktraceDatabase).get().length).toBe(0); + }); + + it('Should load records from the storage provider to context', async () => { + const record: BacktraceDatabaseRecord = { + attachments: [], + count: 1, + data: {} as BacktraceData, + hash: '', + id: '123', + locked: false, + }; + jest.spyOn(testStorageProvider, 'start').mockReturnValue(true); + jest.spyOn(testStorageProvider, 'get').mockResolvedValue(Promise.resolve([record])); + const client = BacktraceTestClient.buildFakeClient( + { + database: testDatabaseSettings, + }, + [], + [], + testStorageProvider, + ); + await new Promise(process.nextTick); + + const [databaseRecord] = (client.database as BacktraceDatabase).get(); + expect(databaseRecord).toStrictEqual(record); + }); + }); +}); diff --git a/packages/sdk-core/tests/database/databaseContextValidationTests.spec.ts b/packages/sdk-core/tests/database/databaseContextValidationTests.spec.ts new file mode 100644 index 00000000..41cfda12 --- /dev/null +++ b/packages/sdk-core/tests/database/databaseContextValidationTests.spec.ts @@ -0,0 +1,64 @@ +import path from 'path'; +import { BacktraceDatabaseConfiguration, BacktraceReportSubmissionResult } from '../../src'; +import { BacktraceDatabase } from '../../src/modules/database/BacktraceDatabase'; +import { BacktraceTestClient } from '../mocks/BacktraceTestClient'; +import { testStorageProvider } from '../mocks/testStorageProvider'; + +describe('Database context validation tests', () => { + describe('Record overflow tests', () => { + const testDatabaseSettings: BacktraceDatabaseConfiguration = { + enabled: true, + autoSend: false, + // this option doesn't matter because we mock the database provider + // interface. However, if bug happen we want to be sure to not create + // anything. Instead we want to fail loud and hard. + createDatabaseDirectory: false, + path: path.join(__dirname, 'database'), + }; + + afterEach(() => { + jest.clearAllMocks(); + }); + + const testingOverflows = [ + [3, 4], + [1, 10], + [3, 1], + ]; + for (const testingOverflow of testingOverflows) { + it(`Should drop the latest record after reaching overflow. Max: ${testingOverflow[0]}. Overflow: ${testingOverflow[1]}`, async () => { + const maximumNumberOfRecords = testingOverflow[0]; + const overflowEvents = testingOverflow[1]; + const client = BacktraceTestClient.buildFakeClient( + { + database: { + ...testDatabaseSettings, + maximumNumberOfRecords, + }, + }, + [], + [], + testStorageProvider, + ); + jest.spyOn(client.requestHandler, 'postError').mockResolvedValue( + Promise.resolve(BacktraceReportSubmissionResult.OnInternalServerError('test')), + ); + const database = client.database as BacktraceDatabase; + if (!database) { + throw new Error('Invalid database setup. Database must be defined!'); + } + + for (let index = 0; index != maximumNumberOfRecords + overflowEvents; index++) { + await client.send(index.toString()); + } + + const records = database.get(); + for (let index = 0; index < maximumNumberOfRecords; index++) { + const record = records[index]; + const expectedMessage = overflowEvents + index; + expect(record.data.attributes['error.message']).toEqual(expectedMessage.toString()); + } + }); + } + }); +}); diff --git a/packages/sdk-core/tests/database/databaseRecordBatchTests.spec.ts b/packages/sdk-core/tests/database/databaseRecordBatchTests.spec.ts new file mode 100644 index 00000000..cc1a0ba0 --- /dev/null +++ b/packages/sdk-core/tests/database/databaseRecordBatchTests.spec.ts @@ -0,0 +1,83 @@ +import path from 'path'; +import { BacktraceReportSubmissionResult } from '../../src'; +import { BacktraceTestClient } from '../mocks/BacktraceTestClient'; +import { testHttpClient } from '../mocks/testHttpClient'; +import { testStorageProvider } from '../mocks/testStorageProvider'; + +jest.useFakeTimers(); + +describe('Database record batch tests', () => { + beforeEach(() => { + jest.setTimeout(60_000); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + it('Should delete batch of reports on maximumRetries unsuccessful tries', async () => { + jest.spyOn(testHttpClient, 'postError').mockResolvedValue( + Promise.resolve(BacktraceReportSubmissionResult.OnInternalServerError('test error')), + ); + const maximumRetries = 3; + const retryInterval = 1000; + const client = BacktraceTestClient.buildFakeClient( + { + database: { + enabled: true, + autoSend: true, + path: path.join(__dirname, 'database'), + maximumRetries, + retryInterval: 1000, + }, + }, + [], + [], + testStorageProvider, + ); + const database = client.database; + if (!database) { + throw new Error('Invalid database setup. Database must be defined!'); + } + + await client.send(new Error('test')); + expect(database.get().length).toEqual(1); + + await jest.advanceTimersByTimeAsync(maximumRetries * retryInterval + 1); + + expect(database.get().length).toEqual(0); + }); + + it('Should not remove the report from the context after less than maximumRetries failures', async () => { + jest.spyOn(testHttpClient, 'postError').mockResolvedValue( + Promise.resolve(BacktraceReportSubmissionResult.OnInternalServerError('test error')), + ); + const maximumRetries = 3; + const retryInterval = 1000; + const client = BacktraceTestClient.buildFakeClient( + { + database: { + enabled: true, + autoSend: true, + path: path.join(__dirname, 'database'), + maximumRetries, + retryInterval: 1000, + }, + }, + [], + [], + testStorageProvider, + ); + const database = client.database; + if (!database) { + throw new Error('Invalid database setup. Database must be defined!'); + } + + await client.send(new Error('test')); + expect(database.get().length).toEqual(1); + + // less than retry intervals + await jest.advanceTimersByTimeAsync(retryInterval); + + expect(database.get().length).toEqual(1); + }); +}); diff --git a/packages/sdk-core/tests/database/databaseSetupTests.spec.ts b/packages/sdk-core/tests/database/databaseSetupTests.spec.ts new file mode 100644 index 00000000..c0b4ab63 --- /dev/null +++ b/packages/sdk-core/tests/database/databaseSetupTests.spec.ts @@ -0,0 +1,117 @@ +import { BacktraceData } from '../../src'; +import { BacktraceReportSubmission } from '../../src/model/http/BacktraceReportSubmission'; +import { BacktraceDatabase } from '../../src/modules/database/BacktraceDatabase'; +import { TEST_SUBMISSION_URL } from '../mocks/BacktraceTestClient'; +import { testHttpClient } from '../mocks/testHttpClient'; +import { testStorageProvider } from '../mocks/testStorageProvider'; + +describe('Database setup tests', () => { + it('The database should be disabled by default', () => { + const database = new BacktraceDatabase( + undefined, + testStorageProvider, + new BacktraceReportSubmission( + { + url: TEST_SUBMISSION_URL, + }, + testHttpClient, + ), + ); + + expect(database.enabled).toBeFalsy(); + }); + + it('Should be enabled after returning true from the start method', () => { + const database = new BacktraceDatabase( + { + autoSend: false, + }, + testStorageProvider, + new BacktraceReportSubmission( + { + url: TEST_SUBMISSION_URL, + }, + testHttpClient, + ), + ); + + const databaseStartResult = database.start(); + + expect(databaseStartResult).toBeTruthy(); + expect(database.enabled).toBeTruthy(); + }); + + it('Should not enable the database if the enable option is set to false', () => { + const database = new BacktraceDatabase( + { enabled: false }, + testStorageProvider, + new BacktraceReportSubmission( + { + url: TEST_SUBMISSION_URL, + }, + testHttpClient, + ), + ); + + const databaseStartResult = database.start(); + + expect(databaseStartResult).toBeFalsy(); + expect(database.enabled).toBeFalsy(); + }); + + it('Should not enable the database if the storage is not prepared', () => { + const database = new BacktraceDatabase( + { + enabled: true, + path: '/path/to/fake/dir', + }, + testStorageProvider, + new BacktraceReportSubmission( + { + url: TEST_SUBMISSION_URL, + }, + testHttpClient, + ), + ); + jest.spyOn(testStorageProvider, 'start').mockReturnValue(false); + + const databaseStartResult = database.start(); + + expect(databaseStartResult).toBeFalsy(); + expect(database.enabled).toBeFalsy(); + }); + + it('Should be disabled after disposing database', () => { + const database = new BacktraceDatabase( + undefined, + testStorageProvider, + new BacktraceReportSubmission( + { + url: TEST_SUBMISSION_URL, + }, + testHttpClient, + ), + ); + + database.start(); + database.dispose(); + + expect(database.enabled).toBeFalsy(); + }); + + it('Should not add a record to disabled database', () => { + const database = new BacktraceDatabase( + undefined, + testStorageProvider, + new BacktraceReportSubmission( + { + url: TEST_SUBMISSION_URL, + }, + testHttpClient, + ), + ); + + const result = database.add({} as BacktraceData, []); + expect(result).toBeFalsy(); + }); +}); diff --git a/packages/sdk-core/tests/database/databaseStorageFlowTests.spec.ts b/packages/sdk-core/tests/database/databaseStorageFlowTests.spec.ts new file mode 100644 index 00000000..baadafef --- /dev/null +++ b/packages/sdk-core/tests/database/databaseStorageFlowTests.spec.ts @@ -0,0 +1,112 @@ +import path from 'path'; +import { BacktraceReportSubmissionResult } from '../../src'; +import { BacktraceDatabase } from '../../src/modules/database/BacktraceDatabase'; +import { BacktraceTestClient } from '../mocks/BacktraceTestClient'; +import { testStorageProvider } from '../mocks/testStorageProvider'; + +describe('Database storage provider flow tests', () => { + const testDatabaseSettings = { + enabled: true, + autoSend: false, + // this option doesn't matter because we mock the database provider + // interface. However, if bug happen we want to be sure to not create + // anything. Instead we want to fail loud and hard. + createDatabaseDirectory: false, + path: path.join(__dirname, 'database'), + }; + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('Setup', () => { + it('Should initialize correctly after database storage initialization', () => { + const client = BacktraceTestClient.buildFakeClient( + { + database: testDatabaseSettings, + }, + [], + [], + testStorageProvider, + ); + const database = client.database as BacktraceDatabase; + if (!database) { + throw new Error('Invalid database setup. Database must be defined!'); + } + + expect(testStorageProvider.start).toHaveBeenCalled(); + expect(database.enabled).toBeTruthy(); + }); + + it('Should not initialize if storage is not setup correctly', () => { + jest.spyOn(testStorageProvider, 'start').mockReturnValueOnce(false); + const client = BacktraceTestClient.buildFakeClient( + { + database: testDatabaseSettings, + }, + [], + [], + testStorageProvider, + ); + const database = client.database as BacktraceDatabase; + if (!database) { + throw new Error('Invalid database setup. Database must be defined!'); + } + + expect(testStorageProvider.start).toHaveBeenCalled(); + expect(database.enabled).toBeFalsy(); + }); + }); + + describe('Add', () => { + it('Should call add on client.send method', async () => { + const testingErrorMessage = 'testingErrorMessage'; + const client = BacktraceTestClient.buildFakeClient( + { + database: testDatabaseSettings, + }, + [], + [], + testStorageProvider, + ); + const database = client.database as BacktraceDatabase; + if (!database) { + throw new Error('Invalid database setup. Database must be defined!'); + } + + jest.spyOn(client.requestHandler, 'postError').mockResolvedValue( + Promise.resolve(BacktraceReportSubmissionResult.OnInternalServerError('test')), + ); + + await client.send(new Error(testingErrorMessage)); + + expect(testStorageProvider.add).toHaveBeenCalled(); + expect(testStorageProvider.delete).not.toHaveBeenCalled(); + }); + + it('Should call delete after successful client.send', async () => { + const testingErrorMessage = 'testingErrorMessage'; + const client = BacktraceTestClient.buildFakeClient( + { + database: testDatabaseSettings, + }, + [], + [], + testStorageProvider, + ); + const database = client.database as BacktraceDatabase; + if (!database) { + throw new Error('Invalid database setup. Database must be defined!'); + } + + jest.spyOn(client.requestHandler, 'postError').mockResolvedValue( + Promise.resolve(BacktraceReportSubmissionResult.Ok({})), + ); + + await client.send(new Error(testingErrorMessage)); + + expect(testStorageProvider.add).toHaveBeenCalled(); + expect(testStorageProvider.delete).toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/sdk-core/tests/mocks/BacktraceTestClient.ts b/packages/sdk-core/tests/mocks/BacktraceTestClient.ts index da8b5749..fab8935c 100644 --- a/packages/sdk-core/tests/mocks/BacktraceTestClient.ts +++ b/packages/sdk-core/tests/mocks/BacktraceTestClient.ts @@ -3,9 +3,10 @@ import { BacktraceAttributeProvider, BacktraceConfiguration, BacktraceCoreClient, - BacktraceReportSubmissionResult, + BacktraceDatabaseStorageProvider, BacktraceRequestHandler, } from '../../src'; +import { testHttpClient } from '../mocks/testHttpClient'; export const TOKEN = '590d39eb154cff1d30f2b689f9a928bb592b25e7e7c10192fe208485ea68d91c'; export const UNIVERSE_NAME = 'test'; export const TEST_SUBMISSION_URL = `https://${UNIVERSE_NAME}.sp.backtrace.io:6098/post?format=json&token=${TOKEN}`; @@ -13,11 +14,13 @@ export const APPLICATION = 'test-app'; export const APPLICATION_VERSION = '5.4.3'; export class BacktraceTestClient extends BacktraceCoreClient { public readonly requestHandler: BacktraceRequestHandler; + public readonly storageProvider?: BacktraceDatabaseStorageProvider; constructor( options: Partial, handler: BacktraceRequestHandler, attributeProviders: BacktraceAttributeProvider[] = [], attachments: BacktraceAttachment[] = [], + storageProvider?: BacktraceDatabaseStorageProvider, ) { super( { @@ -39,14 +42,21 @@ export class BacktraceTestClient extends BacktraceCoreClient { }, handler, attributeProviders, + undefined, + undefined, + undefined, + undefined, + storageProvider, ); this.requestHandler = handler; + this.storageProvider = storageProvider; } public static buildFakeClient( options: Partial = {}, attributeProviders: BacktraceAttributeProvider[] = [], attachments: BacktraceAttachment[] = [], + storageProvider?: BacktraceDatabaseStorageProvider, ) { attributeProviders.push({ type: 'scoped', @@ -57,14 +67,6 @@ export class BacktraceTestClient extends BacktraceCoreClient { }; }, }); - return new BacktraceTestClient( - options, - { - post: jest.fn().mockResolvedValue(Promise.resolve(BacktraceReportSubmissionResult.Ok('Ok'))), - postError: jest.fn().mockResolvedValue(Promise.resolve()), - }, - attributeProviders, - attachments, - ); + return new BacktraceTestClient(options, testHttpClient, attributeProviders, attachments, storageProvider); } } diff --git a/packages/sdk-core/tests/mocks/testStorageProvider.ts b/packages/sdk-core/tests/mocks/testStorageProvider.ts new file mode 100644 index 00000000..f190f7ff --- /dev/null +++ b/packages/sdk-core/tests/mocks/testStorageProvider.ts @@ -0,0 +1,8 @@ +import { BacktraceDatabaseStorageProvider } from '../../src'; + +export const testStorageProvider: BacktraceDatabaseStorageProvider = { + add: jest.fn().mockReturnValue(undefined), + delete: jest.fn().mockResolvedValue(Promise.resolve(true)), + start: jest.fn().mockReturnValue(true), + get: jest.fn().mockResolvedValue(Promise.resolve([])), +}; From 1ddf6b64973f57c39feb3d95893f5bbf02be224e Mon Sep 17 00:00:00 2001 From: Konrad Dysput Date: Tue, 8 Aug 2023 13:32:58 +0200 Subject: [PATCH 2/4] Do not include deduplication model yet --- .../src/modules/database/BacktraceDatabase.ts | 21 ++---------- .../modules/database/DeduplicationModel.ts | 32 ------------------- 2 files changed, 2 insertions(+), 51 deletions(-) delete mode 100644 packages/sdk-core/src/modules/database/DeduplicationModel.ts diff --git a/packages/sdk-core/src/modules/database/BacktraceDatabase.ts b/packages/sdk-core/src/modules/database/BacktraceDatabase.ts index 2ab0446b..52816978 100644 --- a/packages/sdk-core/src/modules/database/BacktraceDatabase.ts +++ b/packages/sdk-core/src/modules/database/BacktraceDatabase.ts @@ -1,14 +1,10 @@ import { IdGenerator } from '../../common/IdGenerator'; import { BacktraceAttachment } from '../../model/attachment'; -import { - BacktraceDatabaseConfiguration, - DeduplicationStrategy, -} from '../../model/configuration/BacktraceDatabaseConfiguration'; +import { BacktraceDatabaseConfiguration } from '../../model/configuration/BacktraceDatabaseConfiguration'; import { BacktraceData } from '../../model/data/BacktraceData'; import { BacktraceReportSubmission } from '../../model/http/BacktraceReportSubmission'; import { BacktraceDatabaseContext } from './BacktraceDatabaseContext'; import { BacktraceDatabaseStorageProvider } from './BacktraceDatabaseStorageProvider'; -import { DeduplicationModel } from './DeduplicationModel'; import { BacktraceDatabaseRecord } from './model/BacktraceDatabaseRecord'; export class BacktraceDatabase { /** @@ -19,7 +15,6 @@ export class BacktraceDatabase { } private readonly _databaseRecordContext: BacktraceDatabaseContext; - private readonly _deduplicationModel: DeduplicationModel; private readonly _maximumRecords: number; private readonly _retryInterval: number; @@ -33,9 +28,6 @@ export class BacktraceDatabase { private readonly _requestHandler: BacktraceReportSubmission, ) { this._databaseRecordContext = new BacktraceDatabaseContext(this._options?.maximumRetries); - this._deduplicationModel = new DeduplicationModel( - this._options?.deduplicationStrategy ?? DeduplicationStrategy.None, - ); this._maximumRecords = this._options?.maximumNumberOfRecords ?? 8; this._retryInterval = this._options?.retryInterval ?? 60_000; } @@ -79,21 +71,12 @@ export class BacktraceDatabase { return undefined; } - const dataHash = this._deduplicationModel.getSha(backtraceData); - if (dataHash) { - const existingRecord = this._databaseRecordContext.find((record) => record.hash === dataHash); - if (existingRecord) { - existingRecord.count++; - return existingRecord; - } - } - this.prepareDatabase(); const record = { count: 1, data: backtraceData, - hash: dataHash, + hash: '', id: IdGenerator.uuid(), locked: false, attachments: attachments, diff --git a/packages/sdk-core/src/modules/database/DeduplicationModel.ts b/packages/sdk-core/src/modules/database/DeduplicationModel.ts deleted file mode 100644 index 65745c81..00000000 --- a/packages/sdk-core/src/modules/database/DeduplicationModel.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { BacktraceData } from '@backtrace/sdk-core/src/model/data/BacktraceData'; -import crypto from 'crypto'; -import { DeduplicationStrategy } from '../..'; - -export class DeduplicationModel { - private readonly CUSTOM_FINGERPRINT_ATTRIBUTE = '_mod_fingerprint'; - constructor(private readonly _deduplicationStrategy: DeduplicationStrategy) {} - - public getSha(data: BacktraceData): string { - const customFingerprint = data.attributes[this.CUSTOM_FINGERPRINT_ATTRIBUTE] as string; - if (customFingerprint) { - return customFingerprint; - } - - if (this._deduplicationStrategy === DeduplicationStrategy.None) { - return ''; - } - - const deduplicationPayload = - ((this._deduplicationStrategy & DeduplicationStrategy.Classifier) == DeduplicationStrategy.Classifier - ? data.classifiers.join(',') - : '') + - ((this._deduplicationStrategy & DeduplicationStrategy.Callstack) == DeduplicationStrategy.Callstack - ? JSON.stringify(data.threads[data.mainThread]) - : '') + - ((this._deduplicationStrategy & DeduplicationStrategy.Message) === DeduplicationStrategy.Message - ? data.attributes['error.message'] - : ''); - - return crypto.createHash('sha256').update(deduplicationPayload).digest('hex'); - } -} From 298568439a2f4a98e01f235734911f438c3dc9d9 Mon Sep 17 00:00:00 2001 From: Konrad Dysput Date: Tue, 8 Aug 2023 13:54:15 +0200 Subject: [PATCH 3/4] Code review adjustements --- .../BacktraceDatabaseFileStorageProvider.ts | 23 ++++++++++++------- .../src/modules/database/BacktraceDatabase.ts | 7 +----- .../database/model/BacktraceDatabaseRecord.ts | 6 ++--- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts b/packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts index 810e7583..caa3fa64 100644 --- a/packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts +++ b/packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts @@ -10,7 +10,7 @@ import { BacktraceDatabaseFileRecord } from './BacktraceDatabaseFileRecord'; export class BacktraceDatabaseFileStorageProvider implements BacktraceDatabaseStorageProvider { private _enabled = true; - private readonly RECORD_SUFFIX = '-recod.json'; + private readonly RECORD_SUFFIX = '-record.json'; private constructor(private readonly _path: string, private readonly _createDatabaseDirectory: boolean = false) {} /** @@ -24,10 +24,15 @@ export class BacktraceDatabaseFileStorageProvider implements BacktraceDatabaseSt if (!options) { return undefined; } - if (!options.enabled || !options.path) { + if (!options.enabled) { return undefined; } + if (options.enabled && !options.path) { + throw new Error( + 'Missing mandatory path to the database. Please define the database.path option in the configuration.', + ); + } return new BacktraceDatabaseFileStorageProvider(options.path, options.createDatabaseDirectory); } @@ -37,13 +42,15 @@ export class BacktraceDatabaseFileStorageProvider implements BacktraceDatabaseSt return false; } - let databaseDirectoryExists = fs.existsSync(this._path); - if (this._createDatabaseDirectory !== false && !databaseDirectoryExists) { - fs.mkdirSync(this._path, { recursive: true }); - databaseDirectoryExists = true; + const databaseDirectoryExists = fs.existsSync(this._path); + if (this._createDatabaseDirectory === false) { + return databaseDirectoryExists; } - - return databaseDirectoryExists; + if (databaseDirectoryExists) { + return true; + } + fs.mkdirSync(this._path, { recursive: true }); + return true; } public async delete(record: BacktraceDatabaseRecord): Promise { diff --git a/packages/sdk-core/src/modules/database/BacktraceDatabase.ts b/packages/sdk-core/src/modules/database/BacktraceDatabase.ts index 52816978..85a7dc99 100644 --- a/packages/sdk-core/src/modules/database/BacktraceDatabase.ts +++ b/packages/sdk-core/src/modules/database/BacktraceDatabase.ts @@ -156,11 +156,7 @@ export class BacktraceDatabase { } private async sendRecords() { - let submissionFailure = false; for (let bucketIndex = 0; bucketIndex < this._databaseRecordContext.bucketCount; bucketIndex++) { - if (submissionFailure) { - break; - } for (const record of this._databaseRecordContext.getBucket(bucketIndex)) { if (record.locked) { continue; @@ -172,9 +168,8 @@ export class BacktraceDatabase { this.remove(record); continue; } - submissionFailure = true; this._databaseRecordContext.increaseBucket(bucketIndex); - break; + return; } finally { record.locked = false; } diff --git a/packages/sdk-core/src/modules/database/model/BacktraceDatabaseRecord.ts b/packages/sdk-core/src/modules/database/model/BacktraceDatabaseRecord.ts index 4f330def..7ca190c2 100644 --- a/packages/sdk-core/src/modules/database/model/BacktraceDatabaseRecord.ts +++ b/packages/sdk-core/src/modules/database/model/BacktraceDatabaseRecord.ts @@ -2,11 +2,11 @@ import { BacktraceAttachment } from '../../../model/attachment'; import { BacktraceData } from '../../../model/data/BacktraceData'; export interface BacktraceDatabaseRecord { - data: BacktraceData; - id: string; + readonly data: BacktraceData; + readonly id: string; + readonly hash: string; attachments: BacktraceAttachment[]; count: number; - hash: string; /** * Determines if the record is in use */ From 41896e51a5628094b4b575d9b28ce45da2aa43e5 Mon Sep 17 00:00:00 2001 From: Konrad Dysput Date: Wed, 9 Aug 2023 12:58:44 +0200 Subject: [PATCH 4/4] Handle potential errors in the storage provider --- .../BacktraceDatabaseFileStorageProvider.ts | 39 ++++++++++++------- packages/sdk-core/src/BacktraceCoreClient.ts | 2 +- .../src/modules/database/BacktraceDatabase.ts | 19 ++++++--- .../database/BacktraceDatabaseContext.ts | 9 +++-- .../BacktraceDatabaseStorageProvider.ts | 4 +- .../tests/mocks/testStorageProvider.ts | 13 +++++-- 6 files changed, 59 insertions(+), 27 deletions(-) diff --git a/packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts b/packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts index caa3fa64..dd957ba6 100644 --- a/packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts +++ b/packages/node/src/database/BacktraceDatabaseFileStorageProvider.ts @@ -53,21 +53,21 @@ export class BacktraceDatabaseFileStorageProvider implements BacktraceDatabaseSt return true; } - public async delete(record: BacktraceDatabaseRecord): Promise { + public delete(record: BacktraceDatabaseRecord): boolean { const recordPath = this.getRecordPath(record.id); - if (!fs.existsSync(recordPath)) { - return false; - } - - await fsPromise.unlink(recordPath); - return true; + return this.unlinkRecord(recordPath); } - public add(record: BacktraceDatabaseRecord): void { + public add(record: BacktraceDatabaseRecord): boolean { const recordPath = this.getRecordPath(record.id); - fs.writeFileSync(recordPath, JSON.stringify(BacktraceDatabaseFileRecord.fromRecord(record)), { - encoding: 'utf8', - }); + try { + fs.writeFileSync(recordPath, JSON.stringify(BacktraceDatabaseFileRecord.fromRecord(record)), { + encoding: 'utf8', + }); + return true; + } catch { + return false; + } } public async get(): Promise { @@ -91,14 +91,27 @@ export class BacktraceDatabaseFileStorageProvider implements BacktraceDatabaseSt continue; } records.push(record); - } catch (err) { - await fsPromise.unlink(recordPath); + } catch { + this.unlinkRecord(recordPath); } } return records; } + private unlinkRecord(recordPath: string): boolean { + if (!fs.existsSync(recordPath)) { + return false; + } + + try { + fs.unlinkSync(recordPath); + return true; + } catch { + return false; + } + } + private getRecordPath(id: string): string { return path.join(this._path, `${id}${this.RECORD_SUFFIX}`); } diff --git a/packages/sdk-core/src/BacktraceCoreClient.ts b/packages/sdk-core/src/BacktraceCoreClient.ts index c4edb227..a4746a84 100644 --- a/packages/sdk-core/src/BacktraceCoreClient.ts +++ b/packages/sdk-core/src/BacktraceCoreClient.ts @@ -211,7 +211,7 @@ export abstract class BacktraceCoreClient { } record.locked = false; if (submissionResult.status === 'Ok') { - await this._database?.remove(record); + this._database?.remove(record); } } diff --git a/packages/sdk-core/src/modules/database/BacktraceDatabase.ts b/packages/sdk-core/src/modules/database/BacktraceDatabase.ts index 85a7dc99..8019ea46 100644 --- a/packages/sdk-core/src/modules/database/BacktraceDatabase.ts +++ b/packages/sdk-core/src/modules/database/BacktraceDatabase.ts @@ -81,7 +81,12 @@ export class BacktraceDatabase { locked: false, attachments: attachments, }; - this._storageProvider.add(record); + + const saveResult = this._storageProvider.add(record); + if (!saveResult) { + return undefined; + } + this._databaseRecordContext.add(record); return record; @@ -115,12 +120,12 @@ export class BacktraceDatabase { * Removes the database record * @param record database records */ - public async remove(record: BacktraceDatabaseRecord) { + public remove(record: BacktraceDatabaseRecord) { if (!this._enabled) { return; } this._databaseRecordContext.remove(record); - await this._storageProvider.delete(record); + this._storageProvider.delete(record); } /** @@ -129,8 +134,12 @@ export class BacktraceDatabase { */ private prepareDatabase(totalNumberOfRecords = 1) { const numberOfRecords = this.count(); - if (numberOfRecords + totalNumberOfRecords > this._maximumRecords) { - this._databaseRecordContext.dropOverflow(totalNumberOfRecords); + if (numberOfRecords + totalNumberOfRecords <= this._maximumRecords) { + return; + } + const recordsToDelete = this._databaseRecordContext.dropOverflow(totalNumberOfRecords); + for (const record of recordsToDelete) { + this._storageProvider.delete(record); } } diff --git a/packages/sdk-core/src/modules/database/BacktraceDatabaseContext.ts b/packages/sdk-core/src/modules/database/BacktraceDatabaseContext.ts index 3a924981..6f7a2b68 100644 --- a/packages/sdk-core/src/modules/database/BacktraceDatabaseContext.ts +++ b/packages/sdk-core/src/modules/database/BacktraceDatabaseContext.ts @@ -73,15 +73,18 @@ export class BacktraceDatabaseContext { } public dropOverflow(overflow: number) { + const result: BacktraceDatabaseRecord[] = []; for (let bucketIndex = this.bucketCount - 1; bucketIndex >= 0; bucketIndex--) { const bucket = this.recordBucket[bucketIndex]; const removedRecords = bucket.splice(0, overflow); - overflow = overflow - removedRecords.length; + result.push(...removedRecords); - if (overflow === 0) { - return; + if (result.length === overflow) { + break; } } + + return result; } private setupRecordBucket(retries: number): BacktraceDatabaseRecord[][] { diff --git a/packages/sdk-core/src/modules/database/BacktraceDatabaseStorageProvider.ts b/packages/sdk-core/src/modules/database/BacktraceDatabaseStorageProvider.ts index 2418b5d9..f9557ebf 100644 --- a/packages/sdk-core/src/modules/database/BacktraceDatabaseStorageProvider.ts +++ b/packages/sdk-core/src/modules/database/BacktraceDatabaseStorageProvider.ts @@ -1,8 +1,8 @@ import { BacktraceDatabaseRecord } from './model/BacktraceDatabaseRecord'; export interface BacktraceDatabaseStorageProvider { - add(databaseRecord: BacktraceDatabaseRecord): void; + add(databaseRecord: BacktraceDatabaseRecord): boolean; get(): Promise; start(): boolean; - delete(record: BacktraceDatabaseRecord): Promise; + delete(record: BacktraceDatabaseRecord): boolean; } diff --git a/packages/sdk-core/tests/mocks/testStorageProvider.ts b/packages/sdk-core/tests/mocks/testStorageProvider.ts index f190f7ff..20a9e02f 100644 --- a/packages/sdk-core/tests/mocks/testStorageProvider.ts +++ b/packages/sdk-core/tests/mocks/testStorageProvider.ts @@ -1,8 +1,15 @@ -import { BacktraceDatabaseStorageProvider } from '../../src'; +import { BacktraceData, BacktraceDatabaseStorageProvider } from '../../src'; export const testStorageProvider: BacktraceDatabaseStorageProvider = { - add: jest.fn().mockReturnValue(undefined), - delete: jest.fn().mockResolvedValue(Promise.resolve(true)), + add: jest.fn().mockReturnValue({ + attachments: [], + count: 1, + data: {} as BacktraceData, + hash: '', + id: '123', + locked: false, + }), + delete: jest.fn().mockReturnValue(true), start: jest.fn().mockReturnValue(true), get: jest.fn().mockResolvedValue(Promise.resolve([])), };