Skip to content

Commit 23c058a

Browse files
committed
node: limit used nodeFs functions, fix unit tests [INT-355]
1 parent 03f4300 commit 23c058a

File tree

9 files changed

+70
-41
lines changed

9 files changed

+70
-41
lines changed

packages/node/src/BacktraceClient.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,14 @@ import { assertDatabasePath } from './database/utils.js';
3636
import { BacktraceStorageModule } from './storage/BacktraceStorage.js';
3737
import { BacktraceStorageModuleFactory } from './storage/BacktraceStorageModuleFactory.js';
3838
import { NodeFsBacktraceStorageModuleFactory } from './storage/NodeFsBacktraceStorage.js';
39+
import { NodeFs } from './storage/nodeFs.js';
3940

4041
export class BacktraceClient extends BacktraceCoreClient<BacktraceConfiguration> {
4142
private _listeners: Record<string, NodeJS.UnhandledRejectionListener | NodeJS.UncaughtExceptionListener> = {};
4243

4344
protected readonly storageFactory: BacktraceStorageModuleFactory;
4445
protected readonly fileAttachmentFactory: BacktraceFileAttachmentFactory;
45-
protected readonly fs: typeof nodeFs;
46+
protected readonly fs: NodeFs;
4647

4748
protected get databaseNodeFsStorage() {
4849
return this.databaseStorage as BacktraceStorageModule | undefined;

packages/node/src/attachment/BacktraceFileAttachment.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@ import { BacktraceAttachment } from '@backtrace/sdk-core';
22
import nodeFs from 'fs';
33
import path from 'path';
44
import { Readable } from 'stream';
5+
import { NodeFs } from '../storage/nodeFs.js';
56

67
export class BacktraceFileAttachment implements BacktraceAttachment<Readable> {
78
public readonly name: string;
89

910
constructor(
1011
public readonly filePath: string,
1112
name?: string,
12-
private readonly _fs: typeof nodeFs = nodeFs,
13+
private readonly _fs: Pick<NodeFs, 'existsSync' | 'createReadStream'> = nodeFs,
1314
) {
1415
this.name = name ?? path.basename(this.filePath);
1516
}
@@ -28,7 +29,7 @@ export interface BacktraceFileAttachmentFactory {
2829
}
2930

3031
export class NodeFsBacktraceFileAttachmentFactory implements BacktraceFileAttachmentFactory {
31-
constructor(private readonly _fs: typeof nodeFs = nodeFs) {}
32+
constructor(private readonly _fs: Pick<NodeFs, 'existsSync' | 'createReadStream'> = nodeFs) {}
3233

3334
public create(filePath: string, name?: string): BacktraceFileAttachment {
3435
return new BacktraceFileAttachment(filePath, name, this._fs);

packages/node/src/builder/BacktraceClientSetup.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import { PartialCoreClientSetup } from '@backtrace/sdk-core';
2-
import nodeFs from 'fs';
32
import { BacktraceSetupConfiguration } from '../BacktraceConfiguration.js';
43
import { BacktraceStorageModule } from '../storage/BacktraceStorage.js';
54
import { BacktraceStorageModuleFactory } from '../storage/BacktraceStorageModuleFactory.js';
5+
import { NodeFs } from '../storage/nodeFs.js';
66

77
export interface BacktraceClientSetup extends PartialCoreClientSetup<'sdkOptions' | 'requestHandler'> {}
88

99
export type BacktraceNodeClientSetup = Omit<BacktraceClientSetup, 'options' | 'database'> & {
1010
readonly options: BacktraceSetupConfiguration;
1111
readonly storageFactory?: BacktraceStorageModuleFactory;
12-
readonly fs?: typeof nodeFs;
12+
readonly fs?: NodeFs;
1313
readonly database?: Omit<NonNullable<BacktraceClientSetup['database']>, 'storage'> & {
1414
readonly storage?: BacktraceStorageModule;
1515
};

packages/node/src/storage/BacktraceStorage.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { BacktraceStorageModule as CoreBacktraceStorageModule } from '@backtrace/sdk-core';
22
import nodeFs from 'fs';
33
import { BacktraceConfiguration } from '../BacktraceConfiguration.js';
4+
import { NodeFs } from './nodeFs.js';
45

56
export interface ReadonlyBacktraceStreamStorage {
67
createReadStream(key: string): nodeFs.ReadStream;
@@ -15,5 +16,5 @@ export type BacktraceStorageModule = CoreBacktraceStorageModule<BacktraceConfigu
1516
export interface BacktraceStorageModuleOptions {
1617
readonly path: string;
1718
readonly createDirectory?: boolean;
18-
readonly fs?: typeof nodeFs;
19+
readonly fs?: NodeFs;
1920
}

packages/node/src/storage/NodeFsBacktraceStorage.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ import nodeFs from 'fs';
22
import path from 'path';
33
import { BacktraceStorageModule, BacktraceStorageModuleOptions } from './BacktraceStorage.js';
44
import { BacktraceStorageModuleFactory } from './BacktraceStorageModuleFactory.js';
5+
import { NodeFs } from './nodeFs.js';
56

67
export class NodeFsBacktraceStorage implements BacktraceStorageModule {
78
private readonly _path: string;
8-
private readonly _fs: typeof nodeFs;
9+
private readonly _fs: NodeFs;
910
private readonly _createDirectory: boolean;
1011

1112
constructor(options: BacktraceStorageModuleOptions) {
@@ -115,11 +116,11 @@ export class NodeFsBacktraceStorage implements BacktraceStorageModule {
115116
}
116117

117118
public createWriteStream(key: string): nodeFs.WriteStream {
118-
return nodeFs.createWriteStream(this.resolvePath(key), 'utf-8');
119+
return this._fs.createWriteStream(this.resolvePath(key), 'utf-8');
119120
}
120121

121122
public createReadStream(key: string): nodeFs.ReadStream {
122-
return nodeFs.createReadStream(this.resolvePath(key), 'utf-8');
123+
return this._fs.createReadStream(this.resolvePath(key), 'utf-8');
123124
}
124125

125126
protected resolvePath(key: string) {
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import nodeFs from 'fs';
2+
3+
/**
4+
* All used methods of the Node file system.
5+
*/
6+
export type NodeFs = Pick<
7+
typeof nodeFs,
8+
| 'mkdirSync'
9+
| 'existsSync'
10+
| 'writeFileSync'
11+
| 'unlinkSync'
12+
| 'readFileSync'
13+
| 'statSync'
14+
| 'readdirSync'
15+
| 'createReadStream'
16+
| 'createWriteStream'
17+
> & {
18+
readonly promises: Pick<(typeof nodeFs)['promises'], 'readdir' | 'stat' | 'writeFile' | 'unlink' | 'readFile'>;
19+
};

packages/node/tests/_mocks/storage.ts

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,25 @@
11
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
22
// @ts-ignore The following import fails due to missing extension, but it cannot have one (it imports a .ts file)
33
import { mockBacktraceStorage } from '@backtrace/sdk-core/tests/_mocks/storage';
4+
5+
import { BacktraceStorageModule } from '@backtrace/sdk-core';
46
import type { MockedBacktraceStorage } from '@backtrace/sdk-core/tests/_mocks/storage.js';
57
import { ReadStream, WriteStream } from 'fs';
68
import { Readable, Writable } from 'stream';
79
import { NodeFsBacktraceStorage } from '../../src/storage/NodeFsBacktraceStorage.js';
10+
import { NodeFs } from '../../src/storage/nodeFs.js';
11+
12+
type MockedFs = Pick<NodeFs, 'existsSync' | 'createReadStream'>;
813

9-
export function mockStreamFileSystem(
14+
export function mockNodeStorageAndFs(
1015
files?: Record<string, string>,
11-
): MockedBacktraceStorage<Omit<NodeFsBacktraceStorage, 'fs' | 'initialize'>> {
12-
const fs = mockBacktraceStorage(files);
16+
): MockedBacktraceStorage<Omit<NodeFsBacktraceStorage & MockedFs, 'fs' | 'initialize'>> {
17+
const storage = mockBacktraceStorage(files) as MockedBacktraceStorage<Omit<BacktraceStorageModule, 'bind'>>;
1318

1419
return {
15-
...fs,
20+
...storage,
1621

17-
ofPath: jest.fn().mockImplementation(() => mockStreamFileSystem()),
22+
existsSync: jest.fn().mockImplementation((p: string) => p in storage.files),
1823

1924
createWriteStream: jest.fn().mockImplementation((p: string) => {
2025
const writable = new Writable({
@@ -25,10 +30,10 @@ export function mockStreamFileSystem(
2530
? chunk
2631
: String(chunk).toString();
2732

28-
if (!fs.files[p]) {
29-
fs.files[p] = str;
33+
if (!storage.files[p]) {
34+
storage.files[p] = str;
3035
} else {
31-
fs.files[p] += str;
36+
storage.files[p] += str;
3237
}
3338

3439
callback && callback();
@@ -39,7 +44,7 @@ export function mockStreamFileSystem(
3944
}),
4045

4146
createReadStream: jest.fn().mockImplementation((p: string) => {
42-
const file = fs.files[p];
47+
const file = storage.files[p];
4348
if (!file) {
4449
throw new Error(`File ${p} does not exist`);
4550
}

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

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import assert from 'assert';
33
import { Readable } from 'stream';
44
import { promisify } from 'util';
55
import { FileBreadcrumbsStorage } from '../../src/breadcrumbs/FileBreadcrumbsStorage.js';
6-
import { mockStreamFileSystem } from '../_mocks/storage.js';
6+
import { NodeFsBacktraceFileAttachmentFactory } from '../../src/index.js';
7+
import { mockNodeStorageAndFs } from '../_mocks/storage.js';
78

89
async function readToEnd(readable: Readable) {
910
return new Promise<Buffer>((resolve, reject) => {
@@ -33,8 +34,8 @@ const nextTick = promisify(process.nextTick);
3334

3435
describe('FileBreadcrumbsStorage', () => {
3536
it('should return added breadcrumbs', async () => {
36-
const fs = mockStreamFileSystem();
37-
const session = new SessionFiles(fs, 'sessionId');
37+
const fs = mockNodeStorageAndFs();
38+
const session = new SessionFiles(fs, { id: 'sessionId', timestamp: Date.now() });
3839

3940
const breadcrumbs: RawBreadcrumb[] = [
4041
{
@@ -86,7 +87,7 @@ describe('FileBreadcrumbsStorage', () => {
8687
},
8788
];
8889

89-
const storage = new FileBreadcrumbsStorage(session, fs, {
90+
const storage = new FileBreadcrumbsStorage(session, fs, new NodeFsBacktraceFileAttachmentFactory(fs), {
9091
maximumBreadcrumbs: 100,
9192
});
9293

@@ -107,8 +108,8 @@ describe('FileBreadcrumbsStorage', () => {
107108
});
108109

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

113114
const breadcrumbs: RawBreadcrumb[] = [
114115
{
@@ -163,7 +164,7 @@ describe('FileBreadcrumbsStorage', () => {
163164
},
164165
];
165166

166-
const storage = new FileBreadcrumbsStorage(session, fs, {
167+
const storage = new FileBreadcrumbsStorage(session, fs, new NodeFsBacktraceFileAttachmentFactory(fs), {
167168
maximumBreadcrumbs: 4,
168169
});
169170

@@ -189,8 +190,8 @@ describe('FileBreadcrumbsStorage', () => {
189190
});
190191

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

195196
const breadcrumbs: RawBreadcrumb[] = [
196197
{
@@ -235,7 +236,7 @@ describe('FileBreadcrumbsStorage', () => {
235236
},
236237
];
237238

238-
const storage = new FileBreadcrumbsStorage(session, fs, {
239+
const storage = new FileBreadcrumbsStorage(session, fs, new NodeFsBacktraceFileAttachmentFactory(fs), {
239240
maximumBreadcrumbs: 2,
240241
});
241242

@@ -261,8 +262,8 @@ describe('FileBreadcrumbsStorage', () => {
261262
});
262263

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

267268
const breadcrumbs: RawBreadcrumb[] = [
268269
{
@@ -302,7 +303,7 @@ describe('FileBreadcrumbsStorage', () => {
302303
},
303304
];
304305

305-
const storage = new FileBreadcrumbsStorage(session, fs, {
306+
const storage = new FileBreadcrumbsStorage(session, fs, new NodeFsBacktraceFileAttachmentFactory(fs), {
306307
maximumBreadcrumbs: 100,
307308
maximumTotalBreadcrumbsSize: JSON.stringify(expectedMain[0]).length + 10,
308309
});
@@ -329,8 +330,8 @@ describe('FileBreadcrumbsStorage', () => {
329330
});
330331

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

335336
const breadcrumbs: RawBreadcrumb[] = [
336337
{
@@ -354,7 +355,7 @@ describe('FileBreadcrumbsStorage', () => {
354355
},
355356
];
356357

357-
const storage = new FileBreadcrumbsStorage(session, fs, {
358+
const storage = new FileBreadcrumbsStorage(session, fs, new NodeFsBacktraceFileAttachmentFactory(fs), {
358359
maximumBreadcrumbs: 4,
359360
});
360361

@@ -373,8 +374,8 @@ describe('FileBreadcrumbsStorage', () => {
373374
});
374375

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

379380
const breadcrumbs: RawBreadcrumb[] = [
380381
{
@@ -398,7 +399,7 @@ describe('FileBreadcrumbsStorage', () => {
398399
},
399400
];
400401

401-
const storage = new FileBreadcrumbsStorage(session, fs, {
402+
const storage = new FileBreadcrumbsStorage(session, fs, new NodeFsBacktraceFileAttachmentFactory(fs), {
402403
maximumBreadcrumbs: 4,
403404
});
404405

packages/node/tests/streams/fileChunkSink.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Writable } from 'stream';
22
import { FileChunkSink } from '../../src/streams/fileChunkSink.js';
3-
import { mockStreamFileSystem } from '../_mocks/storage.js';
3+
import { mockNodeStorageAndFs } from '../_mocks/storage.js';
44

55
function writeAndClose(stream: Writable, value: string) {
66
return new Promise((resolve, reject) => {
@@ -17,7 +17,7 @@ function sortString(a: string, b: string) {
1717

1818
describe('fileChunkSink', () => {
1919
it('should create a filestream with name from filename', async () => {
20-
const fs = mockStreamFileSystem();
20+
const fs = mockNodeStorageAndFs();
2121
const filename = 'abc';
2222
const sink = new FileChunkSink({ file: () => filename, maxFiles: Infinity, storage: fs });
2323

@@ -26,7 +26,7 @@ describe('fileChunkSink', () => {
2626
});
2727

2828
it('should create a filestream each time it is called', async () => {
29-
const fs = mockStreamFileSystem();
29+
const fs = mockNodeStorageAndFs();
3030
const sink = new FileChunkSink({ file: (n) => n.toString(), maxFiles: Infinity, storage: fs });
3131
const expected = [0, 2, 5];
3232

@@ -40,7 +40,7 @@ describe('fileChunkSink', () => {
4040
});
4141

4242
it('should remove previous files if count exceeds maxFiles', async () => {
43-
const fs = mockStreamFileSystem();
43+
const fs = mockNodeStorageAndFs();
4444
const maxFiles = 3;
4545
const sink = new FileChunkSink({ file: (n) => n.toString(), maxFiles, storage: fs });
4646
const files = [0, 2, 5, 6, 79, 81, 38, -1, 3];

0 commit comments

Comments
 (0)