Skip to content

Commit 0ababa9

Browse files
authored
Avoid calling require('lz4') if it's really not required (#316)
This changes the utlity module to return a function instead of the module directly. This way, caller can control when the module is actually tried to be resolved. Signed-off-by: Tuukka Ikkala <tuukka.ikkala@reaktor.com>
1 parent 72d1e80 commit 0ababa9

File tree

5 files changed

+175
-6
lines changed

5 files changed

+175
-6
lines changed

lib/DBSQLSession.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ export default class DBSQLSession implements IDBSQLSession {
232232
}
233233

234234
if (ProtocolVersion.supportsArrowCompression(this.serverProtocolVersion) && request.canDownloadResult !== true) {
235-
request.canDecompressLZ4Result = (options.useLZ4Compression ?? clientConfig.useLZ4Compression) && Boolean(LZ4);
235+
request.canDecompressLZ4Result = (options.useLZ4Compression ?? clientConfig.useLZ4Compression) && Boolean(LZ4());
236236
}
237237

238238
const operationPromise = driver.executeStatement(request);

lib/result/ArrowResultHandler.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export default class ArrowResultHandler implements IResultsProvider<ArrowBatch>
2626
this.arrowSchema = arrowSchema ?? hiveSchemaToArrowSchema(schema);
2727
this.isLZ4Compressed = lz4Compressed ?? false;
2828

29-
if (this.isLZ4Compressed && !LZ4) {
29+
if (this.isLZ4Compressed && !LZ4()) {
3030
throw new HiveDriverError('Cannot handle LZ4 compressed result: module `lz4` not installed');
3131
}
3232
}
@@ -52,7 +52,7 @@ export default class ArrowResultHandler implements IResultsProvider<ArrowBatch>
5252
let totalRowCount = 0;
5353
rowSet?.arrowBatches?.forEach(({ batch, rowCount }) => {
5454
if (batch) {
55-
batches.push(this.isLZ4Compressed ? LZ4!.decode(batch) : batch);
55+
batches.push(this.isLZ4Compressed ? LZ4()!.decode(batch) : batch);
5656
totalRowCount += rowCount.toNumber(true);
5757
}
5858
});

lib/result/CloudFetchResultHandler.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export default class CloudFetchResultHandler implements IResultsProvider<ArrowBa
2727
this.source = source;
2828
this.isLZ4Compressed = lz4Compressed ?? false;
2929

30-
if (this.isLZ4Compressed && !LZ4) {
30+
if (this.isLZ4Compressed && !LZ4()) {
3131
throw new HiveDriverError('Cannot handle LZ4 compressed result: module `lz4` not installed');
3232
}
3333
}
@@ -64,7 +64,7 @@ export default class CloudFetchResultHandler implements IResultsProvider<ArrowBa
6464
}
6565

6666
if (this.isLZ4Compressed) {
67-
batch.batches = batch.batches.map((buffer) => LZ4!.decode(buffer));
67+
batch.batches = batch.batches.map((buffer) => LZ4()!.decode(buffer));
6868
}
6969
return batch;
7070
}

lib/utils/lz4.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ function tryLoadLZ4Module(): LZ4Module | undefined {
77
return require('lz4'); // eslint-disable-line global-require
88
} catch (err) {
99
if (!(err instanceof Error) || !('code' in err)) {
10+
// eslint-disable-next-line no-console
1011
console.warn('Unexpected error loading LZ4 module: Invalid error object', err);
1112
return undefined;
1213
}
@@ -16,14 +17,26 @@ function tryLoadLZ4Module(): LZ4Module | undefined {
1617
}
1718

1819
if (err.code === 'ERR_DLOPEN_FAILED') {
20+
// eslint-disable-next-line no-console
1921
console.warn('LZ4 native module failed to load: Architecture or version mismatch', err);
2022
return undefined;
2123
}
2224

2325
// If it's not a known error, return undefined
26+
// eslint-disable-next-line no-console
2427
console.warn('Unknown error loading LZ4 module: Unhandled error code', err);
2528
return undefined;
2629
}
2730
}
2831

29-
export default tryLoadLZ4Module();
32+
// The null is already tried resolving that failed
33+
let resolvedModule: LZ4Module | null | undefined;
34+
35+
function getResolvedModule() {
36+
if (resolvedModule === undefined) {
37+
resolvedModule = tryLoadLZ4Module() ?? null;
38+
}
39+
return resolvedModule === null ? undefined : resolvedModule;
40+
}
41+
42+
export default getResolvedModule;

tests/unit/utils/lz4.test.ts

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
import { expect } from 'chai';
2+
import sinon from 'sinon';
3+
4+
describe('lz4 module loader', () => {
5+
let moduleLoadStub: sinon.SinonStub | undefined;
6+
let consoleWarnStub: sinon.SinonStub;
7+
8+
beforeEach(() => {
9+
consoleWarnStub = sinon.stub(console, 'warn');
10+
});
11+
12+
afterEach(() => {
13+
consoleWarnStub.restore();
14+
if (moduleLoadStub) {
15+
moduleLoadStub.restore();
16+
}
17+
// Clear module cache
18+
Object.keys(require.cache).forEach((key) => {
19+
if (key.includes('lz4')) {
20+
delete require.cache[key];
21+
}
22+
});
23+
});
24+
25+
const mockModuleLoad = (lz4MockOrError: unknown): { restore: () => void; wasLz4LoadAttempted: () => boolean } => {
26+
// eslint-disable-next-line global-require
27+
const Module = require('module');
28+
const originalLoad = Module._load;
29+
let lz4LoadAttempted = false;
30+
31+
Module._load = (request: string, parent: unknown, isMain: boolean) => {
32+
if (request === 'lz4') {
33+
lz4LoadAttempted = true;
34+
if (lz4MockOrError instanceof Error) {
35+
throw lz4MockOrError;
36+
}
37+
return lz4MockOrError;
38+
}
39+
return originalLoad.call(Module, request, parent, isMain);
40+
};
41+
42+
return {
43+
restore: () => {
44+
Module._load = originalLoad;
45+
},
46+
wasLz4LoadAttempted: () => lz4LoadAttempted,
47+
};
48+
};
49+
50+
const loadLz4Module = () => {
51+
delete require.cache[require.resolve('../../../lib/utils/lz4')];
52+
// eslint-disable-next-line global-require
53+
return require('../../../lib/utils/lz4');
54+
};
55+
56+
it('should successfully load and use lz4 module when available', () => {
57+
const fakeLz4 = {
58+
encode: (buf: Buffer) => {
59+
const compressed = Buffer.from(`compressed:${buf.toString()}`);
60+
return compressed;
61+
},
62+
decode: (buf: Buffer) => {
63+
const decompressed = buf.toString().replace('compressed:', '');
64+
return Buffer.from(decompressed);
65+
},
66+
};
67+
68+
const { restore } = mockModuleLoad(fakeLz4);
69+
const moduleExports = loadLz4Module();
70+
const lz4Module = moduleExports.default();
71+
restore();
72+
73+
expect(lz4Module).to.not.be.undefined;
74+
expect(lz4Module.encode).to.be.a('function');
75+
expect(lz4Module.decode).to.be.a('function');
76+
77+
const testData = Buffer.from('Hello, World!');
78+
const compressed = lz4Module.encode(testData);
79+
const decompressed = lz4Module.decode(compressed);
80+
81+
expect(decompressed.toString()).to.equal('Hello, World!');
82+
expect(consoleWarnStub.called).to.be.false;
83+
});
84+
85+
it('should return undefined when lz4 module fails to load with MODULE_NOT_FOUND', () => {
86+
const err: NodeJS.ErrnoException = new Error("Cannot find module 'lz4'");
87+
err.code = 'MODULE_NOT_FOUND';
88+
89+
const { restore } = mockModuleLoad(err);
90+
const moduleExports = loadLz4Module();
91+
const lz4Module = moduleExports.default();
92+
restore();
93+
94+
expect(lz4Module).to.be.undefined;
95+
expect(consoleWarnStub.called).to.be.false;
96+
});
97+
98+
it('should return undefined and log warning when lz4 fails with ERR_DLOPEN_FAILED', () => {
99+
const err: NodeJS.ErrnoException = new Error('Module did not self-register');
100+
err.code = 'ERR_DLOPEN_FAILED';
101+
102+
const { restore } = mockModuleLoad(err);
103+
const moduleExports = loadLz4Module();
104+
const lz4Module = moduleExports.default();
105+
restore();
106+
107+
expect(lz4Module).to.be.undefined;
108+
expect(consoleWarnStub.calledOnce).to.be.true;
109+
expect(consoleWarnStub.firstCall.args[0]).to.include('Architecture or version mismatch');
110+
});
111+
112+
it('should return undefined and log warning when lz4 fails with unknown error code', () => {
113+
const err: NodeJS.ErrnoException = new Error('Some unknown error');
114+
err.code = 'UNKNOWN_ERROR';
115+
116+
const { restore } = mockModuleLoad(err);
117+
const moduleExports = loadLz4Module();
118+
const lz4Module = moduleExports.default();
119+
restore();
120+
121+
expect(lz4Module).to.be.undefined;
122+
expect(consoleWarnStub.calledOnce).to.be.true;
123+
expect(consoleWarnStub.firstCall.args[0]).to.include('Unhandled error code');
124+
});
125+
126+
it('should return undefined and log warning when error has no code property', () => {
127+
const err = new Error('Error without code');
128+
129+
const { restore } = mockModuleLoad(err);
130+
const moduleExports = loadLz4Module();
131+
const lz4Module = moduleExports.default();
132+
restore();
133+
134+
expect(lz4Module).to.be.undefined;
135+
expect(consoleWarnStub.calledOnce).to.be.true;
136+
expect(consoleWarnStub.firstCall.args[0]).to.include('Invalid error object');
137+
});
138+
139+
it('should not attempt to load lz4 module when getResolvedModule is not called', () => {
140+
const fakeLz4 = {
141+
encode: () => Buffer.from(''),
142+
decode: () => Buffer.from(''),
143+
};
144+
145+
const { restore, wasLz4LoadAttempted } = mockModuleLoad(fakeLz4);
146+
147+
// Load the module but don't call getResolvedModule
148+
loadLz4Module();
149+
// Note: we're NOT calling .default() here
150+
151+
restore();
152+
153+
expect(wasLz4LoadAttempted()).to.be.false;
154+
expect(consoleWarnStub.called).to.be.false;
155+
});
156+
});

0 commit comments

Comments
 (0)