Skip to content

Commit a7eb2c9

Browse files
perf2711perf2711
authored
sdk-core, electron: fix endless loop when loading SessionFiles (#347)
* sdk-core: fix endless loop when loading SessionFiles * electron: fix sessionId build errors * node: fix unit tests (NFC) * react-native: fix unit tests (NFC) * sdk-core: make sessionId backwards compatible
1 parent a36e630 commit a7eb2c9

File tree

11 files changed

+143
-102
lines changed

11 files changed

+143
-102
lines changed

‎packages/electron/src/main/modules/BacktraceMainElectronModule.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,9 @@ export class BacktraceMainElectronModule implements BacktraceModule {
149149
) {
150150
// Sort crashes and sessions by timestamp descending
151151
const crashes = crashReporter.getUploadedReports().sort((a, b) => b.date.getTime() - a.date.getTime());
152-
const previousSessions = session.getPreviousSessions().sort((a, b) => b.timestamp - a.timestamp);
152+
const previousSessions = session
153+
.getPreviousSessions()
154+
.sort((a, b) => b.sessionId.timestamp - a.sessionId.timestamp);
153155

154156
for (const crash of crashes) {
155157
const rxid = this.getCrashRxid(crash.id);
@@ -159,7 +161,7 @@ export class BacktraceMainElectronModule implements BacktraceModule {
159161

160162
try {
161163
// Get first session that happened before the crash
162-
const session = previousSessions.find((p) => p.timestamp < crash.date.getTime());
164+
const session = previousSessions.find((p) => p.sessionId.timestamp < crash.date.getTime());
163165
// If there is no such session, there won't be any other sessions
164166
if (!session) {
165167
break;

‎packages/node/tests/breadcrumbs/FileBreadcrumbsStorage.spec.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const nextTick = promisify(process.nextTick);
3434
describe('FileBreadcrumbsStorage', () => {
3535
it('should return added breadcrumbs', async () => {
3636
const fs = mockStreamFileSystem();
37-
const session = new SessionFiles(fs, '.', 'sessionId');
37+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
3838

3939
const breadcrumbs: RawBreadcrumb[] = [
4040
{
@@ -108,7 +108,7 @@ describe('FileBreadcrumbsStorage', () => {
108108

109109
it('should return added breadcrumbs in two attachments', async () => {
110110
const fs = mockStreamFileSystem();
111-
const session = new SessionFiles(fs, '.', 'sessionId');
111+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
112112

113113
const breadcrumbs: RawBreadcrumb[] = [
114114
{
@@ -190,7 +190,7 @@ describe('FileBreadcrumbsStorage', () => {
190190

191191
it('should return no more than maximumBreadcrumbs breadcrumbs', async () => {
192192
const fs = mockStreamFileSystem();
193-
const session = new SessionFiles(fs, '.', 'sessionId');
193+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
194194

195195
const breadcrumbs: RawBreadcrumb[] = [
196196
{
@@ -262,7 +262,7 @@ describe('FileBreadcrumbsStorage', () => {
262262

263263
it('should return breadcrumbs up to the json size', async () => {
264264
const fs = mockStreamFileSystem();
265-
const session = new SessionFiles(fs, '.', 'sessionId');
265+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
266266

267267
const breadcrumbs: RawBreadcrumb[] = [
268268
{
@@ -330,7 +330,7 @@ describe('FileBreadcrumbsStorage', () => {
330330

331331
it('should return attachments with a valid name from getAttachments', async () => {
332332
const fs = mockStreamFileSystem();
333-
const session = new SessionFiles(fs, '.', 'sessionId');
333+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
334334

335335
const breadcrumbs: RawBreadcrumb[] = [
336336
{
@@ -374,7 +374,7 @@ describe('FileBreadcrumbsStorage', () => {
374374

375375
it('should return attachments with a valid name from getAttachmentProviders', async () => {
376376
const fs = mockStreamFileSystem();
377-
const session = new SessionFiles(fs, '.', 'sessionId');
377+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
378378

379379
const breadcrumbs: RawBreadcrumb[] = [
380380
{

‎packages/react-native/tests/storage/FileBreadcrumbsStorage.spec.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const nextTick = promisify(process.nextTick);
2525
describe('FileBreadcrumbsStorage', () => {
2626
it('should return added breadcrumbs', async () => {
2727
const fs = mockStreamFileSystem();
28-
const session = new SessionFiles(fs, '.', 'sessionId');
28+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
2929

3030
const breadcrumbs: RawBreadcrumb[] = [
3131
{
@@ -99,7 +99,7 @@ describe('FileBreadcrumbsStorage', () => {
9999

100100
it('should return added breadcrumbs in two attachments', async () => {
101101
const fs = mockStreamFileSystem();
102-
const session = new SessionFiles(fs, '.', 'sessionId');
102+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
103103

104104
const breadcrumbs: RawBreadcrumb[] = [
105105
{
@@ -181,7 +181,7 @@ describe('FileBreadcrumbsStorage', () => {
181181

182182
it('should return no more than maximumBreadcrumbs breadcrumbs', async () => {
183183
const fs = mockStreamFileSystem();
184-
const session = new SessionFiles(fs, '.', 'sessionId');
184+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
185185

186186
const breadcrumbs: RawBreadcrumb[] = [
187187
{
@@ -253,7 +253,7 @@ describe('FileBreadcrumbsStorage', () => {
253253

254254
it('should return breadcrumbs up to the json size', async () => {
255255
const fs = mockStreamFileSystem();
256-
const session = new SessionFiles(fs, '.', 'sessionId');
256+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
257257

258258
const breadcrumbs: RawBreadcrumb[] = [
259259
{
@@ -322,7 +322,7 @@ describe('FileBreadcrumbsStorage', () => {
322322

323323
it('should return attachments with a valid name from getAttachments', async () => {
324324
const fs = mockStreamFileSystem();
325-
const session = new SessionFiles(fs, '.', 'sessionId');
325+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
326326

327327
const breadcrumbs: RawBreadcrumb[] = [
328328
{
@@ -366,7 +366,7 @@ describe('FileBreadcrumbsStorage', () => {
366366

367367
it('should return attachments with a valid name from getAttachmentProviders', async () => {
368368
const fs = mockStreamFileSystem();
369-
const session = new SessionFiles(fs, '.', 'sessionId');
369+
const session = new SessionFiles(fs, '.', { id: 'sessionId', timestamp: Date.now() });
370370

371371
const breadcrumbs: RawBreadcrumb[] = [
372372
{

‎packages/sdk-core/src/BacktraceCoreClient.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,10 @@ export abstract class BacktraceCoreClient<
177177
const sessionFiles = new SessionFiles(
178178
this.fileSystem,
179179
this.options.database.path,
180-
this.sessionId,
180+
{
181+
id: this.sessionId,
182+
timestamp: Date.now(),
183+
},
181184
this.options.database.maximumOldSessions ?? 1,
182185
);
183186
this._modules.set(SessionFiles, sessionFiles);

‎packages/sdk-core/src/modules/database/AttachmentBacktraceDatabaseFileRecord.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { BacktraceAttachment } from '../../model/attachment/index.js';
22
import { isFileAttachment } from '../attachments/isFileAttachment.js';
3-
import { FileSystem } from '../storage/index.js';
3+
import { FileSystem, SessionId } from '../storage/index.js';
44
import { BacktraceDatabaseFileRecord } from './BacktraceDatabaseFileRecord.js';
55
import { AttachmentBacktraceDatabaseRecord } from './model/BacktraceDatabaseRecord.js';
66

@@ -10,7 +10,7 @@ export class AttachmentBacktraceDatabaseFileRecord implements AttachmentBacktrac
1010
public readonly rxid: string;
1111
public readonly timestamp: number;
1212
public readonly attachment: BacktraceAttachment<unknown>;
13-
public readonly sessionId: string;
13+
public readonly sessionId: SessionId | string;
1414
public locked: boolean;
1515

1616
private constructor(record: AttachmentBacktraceDatabaseRecord) {

‎packages/sdk-core/src/modules/database/BacktraceDatabase.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { BacktraceDatabaseConfiguration } from '../../model/configuration/Backtr
88
import { BacktraceData } from '../../model/data/BacktraceData.js';
99
import { BacktraceReportSubmission } from '../../model/http/BacktraceReportSubmission.js';
1010
import { BacktraceModule, BacktraceModuleBindData } from '../BacktraceModule.js';
11-
import { SessionFiles } from '../storage/index.js';
11+
import { SessionFiles, SessionId } from '../storage/index.js';
1212
import { BacktraceDatabaseContext } from './BacktraceDatabaseContext.js';
1313
import { BacktraceDatabaseEvents } from './BacktraceDatabaseEvents.js';
1414
import { BacktraceDatabaseStorageProvider } from './BacktraceDatabaseStorageProvider.js';
@@ -135,16 +135,14 @@ export class BacktraceDatabase extends Events<BacktraceDatabaseEvents> implement
135135
return undefined;
136136
}
137137

138-
const sessionId = backtraceData.attributes?.['application.session'];
139-
140138
const record: ReportBacktraceDatabaseRecord = {
141139
type: 'report',
142140
data: backtraceData,
143141
timestamp: TimeHelper.now(),
144142
id: IdGenerator.uuid(),
145143
locked: false,
146144
attachments: attachments,
147-
sessionId: typeof sessionId === 'string' ? sessionId : undefined,
145+
sessionId: this._sessionFiles?.sessionId,
148146
};
149147

150148
this.prepareDatabase([record]);
@@ -171,7 +169,7 @@ export class BacktraceDatabase extends Events<BacktraceDatabaseEvents> implement
171169
public addAttachment(
172170
rxid: string,
173171
attachment: BacktraceAttachment,
174-
sessionId: string,
172+
sessionId: SessionId,
175173
): AttachmentBacktraceDatabaseRecord | undefined {
176174
if (!this._enabled) {
177175
return undefined;
@@ -398,7 +396,7 @@ export class BacktraceDatabase extends Events<BacktraceDatabaseEvents> implement
398396
}
399397

400398
const sessionId = record.sessionId;
401-
if (typeof sessionId !== 'string') {
399+
if (!SessionFiles.isValidSessionId(sessionId)) {
402400
this._sessionFiles.lockPreviousSessions(record.id);
403401
return;
404402
}

‎packages/sdk-core/src/modules/database/ReportBacktraceDatabaseFileRecord.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { BacktraceAttachment } from '../../model/attachment/index.js';
22
import { BacktraceData } from '../../model/data/index.js';
33
import { isFileAttachment } from '../attachments/isFileAttachment.js';
4-
import { FileSystem } from '../storage/index.js';
4+
import { FileSystem, SessionId } from '../storage/index.js';
55
import { BacktraceDatabaseFileRecord } from './BacktraceDatabaseFileRecord.js';
66
import { ReportBacktraceDatabaseRecord } from './model/BacktraceDatabaseRecord.js';
77

@@ -10,7 +10,7 @@ export class ReportBacktraceDatabaseFileRecord implements ReportBacktraceDatabas
1010
public readonly data: BacktraceData;
1111
public readonly id: string;
1212
public readonly timestamp: number;
13-
public readonly sessionId?: string;
13+
public readonly sessionId?: SessionId | string;
1414
public locked: boolean;
1515

1616
private constructor(

‎packages/sdk-core/src/modules/database/model/BacktraceDatabaseRecord.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { BacktraceAttachment } from '../../../model/attachment/index.js';
22
import { BacktraceData } from '../../../model/data/BacktraceData.js';
3+
import { SessionId } from '../../storage/SessionFiles.js';
34

45
export interface ReportBacktraceDatabaseRecord {
56
readonly type: 'report';
67
readonly data: BacktraceData;
78
readonly id: string;
89
readonly timestamp: number;
9-
readonly sessionId?: string;
10+
readonly sessionId?: string | SessionId;
1011
attachments: BacktraceAttachment[];
1112
/**
1213
* Determines if the record is in use
@@ -20,7 +21,7 @@ export interface AttachmentBacktraceDatabaseRecord {
2021
readonly rxid: string;
2122
readonly timestamp: number;
2223
readonly attachment: BacktraceAttachment;
23-
readonly sessionId: string;
24+
readonly sessionId: string | SessionId;
2425
/**
2526
* Determines if the record is in use
2627
*/

‎packages/sdk-core/src/modules/storage/SessionFiles.ts‎

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,19 @@ import { FileSystem } from './FileSystem.js';
55

66
interface FileSession {
77
readonly file: string;
8-
readonly sessionId: string;
8+
readonly sessionId: SessionId;
99
readonly escapedSessionId: string;
10-
readonly timestamp: number;
1110
}
1211

1312
type SessionEvents = {
1413
unlocked: [];
1514
};
1615

16+
export interface SessionId {
17+
readonly id: string;
18+
readonly timestamp: number;
19+
}
20+
1721
const SESSION_MARKER_PREFIX = 'bt-session';
1822

1923
const isDefined = <T>(t: T | undefined): t is T => !!t;
@@ -30,17 +34,16 @@ export class SessionFiles implements BacktraceModule {
3034
constructor(
3135
private readonly _fileSystem: FileSystem,
3236
private readonly _directory: string,
33-
public readonly sessionId: string,
37+
public readonly sessionId: SessionId,
3438
private readonly _maxPreviousLockedSessions = 1,
35-
public readonly timestamp = Date.now(),
3639
private readonly _lockable = true,
3740
) {
38-
this._escapedSessionId = this.escapeFileName(sessionId);
41+
this._escapedSessionId = this.escapeFileName(sessionId.id);
3942
this.marker = this.getFileName(SESSION_MARKER_PREFIX);
4043
}
4144

4245
public initialize(): void {
43-
this.createSessionMarker();
46+
return this.createSessionMarker();
4447
}
4548

4649
public getPreviousSession() {
@@ -57,12 +60,14 @@ export class SessionFiles implements BacktraceModule {
5760
.filter((f) => f.startsWith(SESSION_MARKER_PREFIX))
5861
.map((f) => this.getFileSession(f))
5962
.filter(isDefined)
60-
.sort((a, b) => b.timestamp - a.timestamp);
63+
.sort((a, b) => b.sessionId.timestamp - a.sessionId.timestamp);
6164

62-
const currentSessionMarker = sessionMarkers.find((s) => s.sessionId === this.sessionId);
65+
const currentSessionMarker = sessionMarkers.find((s) => this.sessionIdEquals(s.sessionId, this.sessionId));
6366

6467
const lastSessionMarker = currentSessionMarker
65-
? sessionMarkers.filter(({ timestamp }) => currentSessionMarker.timestamp > timestamp)[0]
68+
? sessionMarkers.filter(
69+
({ sessionId }) => currentSessionMarker.sessionId.timestamp > sessionId.timestamp,
70+
)[0]
6671
: sessionMarkers[0];
6772

6873
if (!lastSessionMarker) {
@@ -74,15 +79,14 @@ export class SessionFiles implements BacktraceModule {
7479
this._directory,
7580
lastSessionMarker.sessionId,
7681
this._maxPreviousLockedSessions - 1,
77-
lastSessionMarker.timestamp,
7882
this._maxPreviousLockedSessions > 0,
7983
));
8084
}
8185

82-
public getSessionWithId(sessionId: string) {
86+
public getSessionWithId(sessionId: SessionId) {
8387
// eslint-disable-next-line @typescript-eslint/no-this-alias
8488
let session: SessionFiles | undefined = this;
85-
while (session && session.sessionId !== sessionId) {
89+
while (session && this.sessionIdEquals(session.sessionId, sessionId)) {
8690
session = session.getPreviousSession();
8791
}
8892

@@ -124,7 +128,11 @@ export class SessionFiles implements BacktraceModule {
124128
public getFileName(prefix: string) {
125129
this.throwIfCleared();
126130

127-
return this._directory + '/' + `${this.escapeFileName(prefix)}_${this._escapedSessionId}_${this.timestamp}`;
131+
return (
132+
this._directory +
133+
'/' +
134+
`${this.escapeFileName(prefix)}_${this._escapedSessionId}_${this.sessionId.timestamp}`
135+
);
128136
}
129137

130138
public getSessionFiles() {
@@ -134,7 +142,7 @@ export class SessionFiles implements BacktraceModule {
134142
return files
135143
.map((file) => this.getFileSession(file))
136144
.filter(isDefined)
137-
.filter(({ sessionId }) => sessionId === this.sessionId)
145+
.filter(({ sessionId }) => this.sessionIdEquals(sessionId, this.sessionId))
138146
.map(({ file }) => this._directory + '/' + file);
139147
}
140148

@@ -184,7 +192,14 @@ export class SessionFiles implements BacktraceModule {
184192
return undefined;
185193
}
186194

187-
return { file, escapedSessionId, timestamp, sessionId: this.unescapeFileName(escapedSessionId) };
195+
return {
196+
file,
197+
escapedSessionId,
198+
sessionId: {
199+
timestamp,
200+
id: this.unescapeFileName(escapedSessionId),
201+
},
202+
};
188203
}
189204

190205
private readDirectoryFiles() {
@@ -232,4 +247,19 @@ export class SessionFiles implements BacktraceModule {
232247
throw new Error('This session files are cleared.');
233248
}
234249
}
250+
251+
private sessionIdEquals(a: SessionId, b: SessionId) {
252+
return a.id === b.id && a.timestamp === b.timestamp;
253+
}
254+
255+
public static isValidSessionId(value: unknown): value is SessionId {
256+
return (
257+
typeof value === 'object' &&
258+
!!value &&
259+
'id' in value &&
260+
'timestamp' in value &&
261+
typeof value.id === 'string' &&
262+
typeof value.timestamp === 'number'
263+
);
264+
}
235265
}

‎packages/sdk-core/tests/database/databaseSetupTests.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ function randomAttachmentRecord(): AttachmentBacktraceDatabaseRecord {
2828
timestamp: Date.now(),
2929
attachment: { name: 'x', get: () => undefined },
3030
rxid: 'x',
31-
sessionId: crypto.randomUUID(),
31+
sessionId: { id: crypto.randomUUID(), timestamp: Date.now() },
3232
};
3333
}
3434

0 commit comments

Comments
 (0)