Skip to content

Commit 865e658

Browse files
nbbeekendurran
andauthored
fix(NODE-4977): load snappy lazily (#3726)
Co-authored-by: Durran Jordan <[email protected]>
1 parent dda4ec0 commit 865e658

File tree

6 files changed

+187
-20
lines changed

6 files changed

+187
-20
lines changed

src/cmap/wire_protocol/compression.ts

+14-7
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { promisify } from 'util';
22
import * as zlib from 'zlib';
33

44
import { LEGACY_HELLO_COMMAND } from '../../constants';
5-
import { getZstdLibrary, Snappy, type ZStandard } from '../../deps';
5+
import { getSnappy, getZstdLibrary, type SnappyLib, type ZStandard } from '../../deps';
66
import { MongoDecompressionError, MongoInvalidArgumentError } from '../../error';
77

88
/** @public */
@@ -38,6 +38,17 @@ const zlibInflate = promisify(zlib.inflate.bind(zlib));
3838
const zlibDeflate = promisify(zlib.deflate.bind(zlib));
3939

4040
let zstd: typeof ZStandard;
41+
let Snappy: SnappyLib | null = null;
42+
function loadSnappy() {
43+
if (Snappy == null) {
44+
const snappyImport = getSnappy();
45+
if ('kModuleError' in snappyImport) {
46+
throw snappyImport.kModuleError;
47+
}
48+
Snappy = snappyImport;
49+
}
50+
return Snappy;
51+
}
4152

4253
// Facilitate compressing a message using an agreed compressor
4354
export async function compress(
@@ -47,9 +58,7 @@ export async function compress(
4758
const zlibOptions = {} as zlib.ZlibOptions;
4859
switch (options.agreedCompressor) {
4960
case 'snappy': {
50-
if ('kModuleError' in Snappy) {
51-
throw Snappy['kModuleError'];
52-
}
61+
Snappy ??= loadSnappy();
5362
return Snappy.compress(dataToBeCompressed);
5463
}
5564
case 'zstd': {
@@ -88,9 +97,7 @@ export async function decompress(compressorID: number, compressedData: Buffer):
8897

8998
switch (compressorID) {
9099
case Compressor.snappy: {
91-
if ('kModuleError' in Snappy) {
92-
throw Snappy['kModuleError'];
93-
}
100+
Snappy ??= loadSnappy();
94101
return Snappy.uncompress(compressedData, { asBuffer: true });
95102
}
96103
case Compressor.zstd: {

src/deps.ts

+15-11
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ export function getAwsCredentialProvider():
9797
}
9898
}
9999

100-
type SnappyLib = {
100+
/** @internal */
101+
export type SnappyLib = {
101102
/**
102103
* In order to support both we must check the return value of the function
103104
* @param buf - Buffer to be compressed
@@ -111,16 +112,19 @@ type SnappyLib = {
111112
uncompress(buf: Buffer, opt: { asBuffer: true }): Promise<Buffer>;
112113
};
113114

114-
export let Snappy: SnappyLib | { kModuleError: MongoMissingDependencyError } = makeErrorModule(
115-
new MongoMissingDependencyError(
116-
'Optional module `snappy` not found. Please install it to enable snappy compression'
117-
)
118-
);
119-
120-
try {
121-
// Ensure you always wrap an optional require in the try block NODE-3199
122-
Snappy = require('snappy');
123-
} catch {} // eslint-disable-line
115+
export function getSnappy(): SnappyLib | { kModuleError: MongoMissingDependencyError } {
116+
try {
117+
// Ensure you always wrap an optional require in the try block NODE-3199
118+
const value = require('snappy');
119+
return value;
120+
} catch (cause) {
121+
const kModuleError = new MongoMissingDependencyError(
122+
'Optional module `snappy` not found. Please install it to enable snappy compression',
123+
{ cause }
124+
);
125+
return { kModuleError };
126+
}
127+
}
124128

125129
export let saslprep: typeof import('saslprep') | { kModuleError: MongoMissingDependencyError } =
126130
makeErrorModule(

src/error.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -692,8 +692,9 @@ export class MongoMissingCredentialsError extends MongoAPIError {
692692
* @category Error
693693
*/
694694
export class MongoMissingDependencyError extends MongoAPIError {
695-
constructor(message: string) {
695+
constructor(message: string, { cause }: { cause?: Error } = {}) {
696696
super(message);
697+
if (cause) this.cause = cause;
697698
}
698699

699700
override get name(): string {

test/action/dependency.test.ts

+74-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import * as path from 'node:path';
55
import { expect } from 'chai';
66

77
import { dependencies, peerDependencies, peerDependenciesMeta } from '../../package.json';
8+
import { itInNodeProcess } from '../tools/utils';
89

910
const EXPECTED_DEPENDENCIES = ['bson', 'mongodb-connection-string-url', 'socks'];
1011
const EXPECTED_PEER_DEPENDENCIES = [
@@ -65,11 +66,24 @@ describe('package.json', function () {
6566

6667
expect(result).to.include('import success!');
6768
});
69+
70+
if (depName === 'snappy') {
71+
itInNodeProcess(
72+
'getSnappy returns rejected import',
73+
async function ({ expect, mongodb }) {
74+
const snappyImport = mongodb.getSnappy();
75+
expect(snappyImport).to.have.nested.property(
76+
'kModuleError.name',
77+
'MongoMissingDependencyError'
78+
);
79+
}
80+
);
81+
}
6882
});
6983

7084
context(`when ${depName} is installed`, () => {
7185
beforeEach(async () => {
72-
execSync(`npm install --no-save "${depName}"@${depMajor}`);
86+
execSync(`npm install --no-save "${depName}"@"${depMajor}"`);
7387
});
7488

7589
it(`driver is importable`, () => {
@@ -81,7 +95,66 @@ describe('package.json', function () {
8195

8296
expect(result).to.include('import success!');
8397
});
98+
99+
if (depName === 'snappy') {
100+
itInNodeProcess(
101+
'getSnappy returns fulfilled import',
102+
async function ({ expect, mongodb }) {
103+
const snappyImport = mongodb.getSnappy();
104+
expect(snappyImport).to.have.property('compress').that.is.a('function');
105+
expect(snappyImport).to.have.property('uncompress').that.is.a('function');
106+
}
107+
);
108+
}
84109
});
85110
}
86111
});
112+
113+
const EXPECTED_IMPORTS = [
114+
'bson',
115+
'saslprep',
116+
'sparse-bitfield',
117+
'memory-pager',
118+
'mongodb-connection-string-url',
119+
'whatwg-url',
120+
'webidl-conversions',
121+
'tr46',
122+
'socks',
123+
'ip',
124+
'smart-buffer'
125+
];
126+
127+
describe('mongodb imports', () => {
128+
let imports: string[];
129+
beforeEach(async function () {
130+
for (const key of Object.keys(require.cache)) delete require.cache[key];
131+
require('../../src');
132+
imports = Array.from(
133+
new Set(
134+
Object.entries(require.cache)
135+
.filter(([modKey]) => modKey.includes('/node_modules/'))
136+
.map(([modKey]) => {
137+
const leadingPkgName = modKey.split('/node_modules/')[1];
138+
const [orgName, pkgName] = leadingPkgName.split('/');
139+
if (orgName.startsWith('@')) {
140+
return `${orgName}/${pkgName}`;
141+
}
142+
return orgName;
143+
})
144+
)
145+
);
146+
});
147+
148+
context('when importing mongodb', () => {
149+
it('only contains the expected imports', function () {
150+
expect(imports).to.deep.equal(EXPECTED_IMPORTS);
151+
});
152+
153+
it('does not import optional dependencies', () => {
154+
for (const peerDependency of EXPECTED_PEER_DEPENDENCIES) {
155+
expect(imports).to.not.include(peerDependency);
156+
}
157+
});
158+
});
159+
});
87160
});

test/tools/utils.ts

+65
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
import * as child_process from 'node:child_process';
2+
import { once } from 'node:events';
3+
import * as fs from 'node:fs/promises';
4+
import * as path from 'node:path';
5+
16
import { EJSON } from 'bson';
27
import * as BSON from 'bson';
38
import { expect } from 'chai';
@@ -499,3 +504,63 @@ export function topologyWithPlaceholderClient(
499504
options as TopologyOptions
500505
);
501506
}
507+
508+
export async function itInNodeProcess(
509+
title: string,
510+
fn: (d: { expect: typeof import('chai').expect; mongodb: typeof import('../mongodb') }) => void
511+
) {
512+
it(title, async () => {
513+
const script = `
514+
import { expect } from 'chai';
515+
import * as mongodb from './test/mongodb';
516+
const run = ${fn};
517+
run({ expect, mongodb }).then(
518+
() => {
519+
process.exitCode = 0;
520+
},
521+
error => {
522+
console.error(error)
523+
process.exitCode = 1;
524+
}
525+
);\n`;
526+
527+
const scriptName = `./testing_${title.split(/\s/).join('_')}_script.cts`;
528+
const cwd = path.resolve(__dirname, '..', '..');
529+
const tsNode = path.resolve(__dirname, '..', '..', 'node_modules', '.bin', 'ts-node');
530+
try {
531+
await fs.writeFile(scriptName, script, { encoding: 'utf8' });
532+
const scriptInstance = child_process.fork(scriptName, {
533+
signal: AbortSignal.timeout(50_000),
534+
cwd,
535+
stdio: 'pipe',
536+
execArgv: [tsNode]
537+
});
538+
539+
scriptInstance.stdout?.setEncoding('utf8');
540+
scriptInstance.stderr?.setEncoding('utf8');
541+
542+
let stdout = '';
543+
scriptInstance.stdout?.addListener('data', data => {
544+
stdout += data;
545+
});
546+
547+
let stderr = '';
548+
scriptInstance.stderr?.addListener('data', (data: string) => {
549+
stderr += data;
550+
});
551+
552+
// do not fail the test if the debugger is running
553+
stderr = stderr
554+
.split('\n')
555+
.filter(line => !line.startsWith('Debugger') && !line.startsWith('For help'))
556+
.join('\n');
557+
558+
const [exitCode] = await once(scriptInstance, 'close');
559+
560+
if (stderr.length) console.log(stderr);
561+
expect({ exitCode, stdout, stderr }).to.deep.equal({ exitCode: 0, stdout: '', stderr: '' });
562+
} finally {
563+
await fs.unlink(scriptName);
564+
}
565+
});
566+
}

test/unit/error.test.ts

+17
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
MONGODB_ERROR_CODES,
1717
MongoError,
1818
MongoErrorLabel,
19+
MongoMissingDependencyError,
1920
MongoNetworkError,
2021
MongoNetworkTimeoutError,
2122
MongoParseError,
@@ -155,6 +156,22 @@ describe('MongoErrors', () => {
155156
});
156157
});
157158

159+
describe('MongoMissingDependencyError#constructor', () => {
160+
context('when options.cause is set', () => {
161+
it('attaches the cause property to the instance', () => {
162+
const error = new MongoMissingDependencyError('missing!', { cause: new Error('hello') });
163+
expect(error).to.have.property('cause');
164+
});
165+
});
166+
167+
context('when options.cause is not set', () => {
168+
it('attaches the cause property to the instance', () => {
169+
const error = new MongoMissingDependencyError('missing!', { cause: undefined });
170+
expect(error).to.not.have.property('cause');
171+
});
172+
});
173+
});
174+
158175
describe('#isSDAMUnrecoverableError', function () {
159176
context('when the error is a MongoParseError', function () {
160177
it('returns true', function () {

0 commit comments

Comments
 (0)