Skip to content

Commit 8fd53a4

Browse files
requested changes: remove log calls, change chai to expect, clarify script vars/functions
1 parent 133b20d commit 8fd53a4

File tree

3 files changed

+59
-51
lines changed

3 files changed

+59
-51
lines changed

test/integration/node-specific/client_close.test.ts

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,13 @@ describe.skip('MongoClient.close() Integration', () => {
1818
await runScriptAndGetProcessInfo(
1919
'tls-file-read',
2020
config,
21-
async function run({ MongoClient, uri, log, chai }) {
21+
async function run({ MongoClient, uri, expect }) {
2222
const infiniteFile = '/dev/zero';
2323
const client = new MongoClient(uri, { tlsCertificateKeyFile: infiniteFile });
2424
client.connect();
25-
log({ ActiveResources: process.getActiveResourcesInfo() });
26-
chai.expect(process.getActiveResourcesInfo()).to.include('FSReqPromise');
25+
expect(process.getActiveResourcesInfo()).to.include('FSReqPromise');
2726
await client.close();
28-
chai.expect(process.getActiveResourcesInfo()).to.not.include('FSReqPromise');
27+
expect(process.getActiveResourcesInfo()).to.not.include('FSReqPromise');
2928
}
3029
);
3130
});
@@ -60,7 +59,7 @@ describe.skip('MongoClient.close() Integration', () => {
6059
await runScriptAndGetProcessInfo(
6160
'token-file-read',
6261
config,
63-
async function run({ MongoClient, uri, chai }) {
62+
async function run({ MongoClient, uri, expect }) {
6463
const infiniteFile = '/dev/zero';
6564
process.env.OIDC_TOKEN_FILE = infiniteFile;
6665
const options = {
@@ -69,9 +68,9 @@ describe.skip('MongoClient.close() Integration', () => {
6968
};
7069
const client = new MongoClient(uri, options);
7170
client.connect();
72-
chai.expect(process.getActiveResourcesInfo()).to.include('FSReqPromise');
71+
expect(process.getActiveResourcesInfo()).to.include('FSReqPromise');
7372
await client.close();
74-
chai.expect(process.getActiveResourcesInfo()).to.not.include('FSReqPromise');
73+
expect(process.getActiveResourcesInfo()).to.not.include('FSReqPromise');
7574
}
7675
);
7776
});
@@ -232,7 +231,7 @@ describe.skip('MongoClient.close() Integration', () => {
232231
await runScriptAndGetProcessInfo(
233232
'tls-file-read-auto-encryption',
234233
config,
235-
async function run({ MongoClient, uri, log, chai, ClientEncryption, BSON }) {
234+
async function run({ MongoClient, uri, expect, ClientEncryption, BSON }) {
236235
const infiniteFile = '/dev/zero';
237236

238237
const kmsProviders = BSON.EJSON.parse(process.env.CSFLE_KMS_PROVIDERS);
@@ -296,18 +295,16 @@ describe.skip('MongoClient.close() Integration', () => {
296295
.collection('coll')
297296
.insertOne({ a: 1 });
298297

299-
chai.expect(process.getActiveResourcesInfo()).to.include('FSReqPromise');
300-
log({ activeResourcesBeforeClose: process.getActiveResourcesInfo() });
298+
expect(process.getActiveResourcesInfo()).to.include('FSReqPromise');
301299

302300
await keyVaultClient.close();
303301
await encryptedClient.close();
304302

305-
chai.expect(process.getActiveResourcesInfo()).to.not.include('FSReqPromise');
306-
log({ activeResourcesAfterClose: process.getActiveResourcesInfo() });
303+
expect(process.getActiveResourcesInfo()).to.not.include('FSReqPromise');
307304

308305
const err = await insertPromise.catch(e => e);
309-
chai.expect(err).to.exist;
310-
chai.expect(err.errmsg).to.contain('Error in KMS response');
306+
expect(err).to.exist;
307+
expect(err.errmsg).to.contain('Error in KMS response');
311308
}
312309
);
313310
});
@@ -328,7 +325,7 @@ describe.skip('MongoClient.close() Integration', () => {
328325
await runScriptAndGetProcessInfo(
329326
'tls-file-read-client-encryption',
330327
config,
331-
async function run({ MongoClient, uri, log, chai, ClientEncryption, BSON }) {
328+
async function run({ MongoClient, uri, expect, ClientEncryption, BSON }) {
332329
const infiniteFile = '/dev/zero';
333330
const kmsProviders = BSON.EJSON.parse(process.env.CSFLE_KMS_PROVIDERS);
334331
const masterKey = {
@@ -349,19 +346,15 @@ describe.skip('MongoClient.close() Integration', () => {
349346

350347
const dataKeyPromise = clientEncryption.createDataKey(provider, { masterKey });
351348

352-
chai.expect(process.getActiveResourcesInfo()).to.include('FSReqPromise');
353-
354-
log({ activeResourcesBeforeClose: process.getActiveResourcesInfo() });
349+
expect(process.getActiveResourcesInfo()).to.include('FSReqPromise');
355350

356351
await keyVaultClient.close();
357352

358-
chai.expect(process.getActiveResourcesInfo()).to.not.include('FSReqPromise');
359-
360-
log({ activeResourcesAfterClose: process.getActiveResourcesInfo() });
353+
expect(process.getActiveResourcesInfo()).to.not.include('FSReqPromise');
361354

362355
const err = await dataKeyPromise.catch(e => e);
363-
chai.expect(err).to.exist;
364-
chai.expect(err.errmsg).to.contain('Error in KMS response');
356+
expect(err).to.exist;
357+
expect(err.errmsg).to.contain('Error in KMS response');
365358
}
366359
);
367360
});

test/integration/node-specific/resource_tracking_script_builder.ts

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ export type HeapResourceTestFunction = (options: {
2020
export type ProcessResourceTestFunction = (options: {
2121
MongoClient: typeof MongoClient;
2222
uri: string;
23-
log: (out: any) => void;
24-
chai: { expect: typeof expect };
23+
log?: (out: any) => void;
24+
expect: typeof expect;
2525
ClientEncryption?: typeof ClientEncryption;
2626
BSON?: typeof BSON;
2727
}) => Promise<void>;
@@ -47,7 +47,7 @@ export async function testScriptFactory(
4747

4848
resourceScript = resourceScript.replace('DRIVER_SOURCE_PATH', DRIVER_SRC_PATH);
4949
resourceScript = resourceScript.replace('FUNCTION_STRING', `(${func.toString()})`);
50-
resourceScript = resourceScript.replace('NAME_STRING', JSON.stringify(name));
50+
resourceScript = resourceScript.replace('SCRIPT_NAME_STRING', JSON.stringify(name));
5151
resourceScript = resourceScript.replace('URI_STRING', JSON.stringify(uri));
5252
resourceScript = resourceScript.replace('ITERATIONS_STRING', `${iterations}`);
5353

@@ -140,11 +140,11 @@ export async function runScriptAndReturnHeapInfo(
140140
* **The provided function is run in an isolated Node.js process**
141141
*
142142
* A user of this function will likely need to familiarize themselves with the surrounding scripting, but briefly:
143-
* - Every MongoClient you construct should have an asyncResource attached to it like so:
144-
* ```js
145-
* mongoClient.asyncResource = new this.async_hooks.AsyncResource('MongoClient');
146-
* ```
147-
* - You can perform any number of operations and connects/closes of MongoClients
143+
* - Many MongoClient operations (construction, connection, commands) can result in resources that keep the JS event loop running.
144+
* - Timers
145+
* - Active Sockets
146+
* - File Read Hangs
147+
*
148148
* - This function performs assertions that at the end of the provided function, the js event loop has been exhausted
149149
*
150150
* @param name - the name of the script, this defines the name of the file, it will be cleaned up if the function returns successfully
@@ -168,16 +168,8 @@ export async function runScriptAndGetProcessInfo(
168168
await writeFile(scriptName, scriptContent, { encoding: 'utf8' });
169169
const logFile = 'logs.txt';
170170

171-
const processDiedController = new AbortController();
172171
const script = spawn(process.argv[0], [scriptName], { stdio: ['ignore', 'ignore', 'ignore'] });
173172

174-
// Interrupt our awaiting of messages if the process crashed
175-
script.once('close', exitCode => {
176-
if (exitCode !== 0) {
177-
processDiedController.abort(new Error(`process exited with: ${exitCode}`));
178-
}
179-
});
180-
181173
const willClose = once(script, 'close');
182174

183175
// make sure the process ended

test/tools/fixtures/process_resource_script.in.js

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,36 +4,62 @@
44
/* eslint-disable no-unused-vars */
55
const driverPath = DRIVER_SOURCE_PATH;
66
const func = FUNCTION_STRING;
7-
const name = NAME_STRING;
7+
const scriptName = SCRIPT_NAME_STRING;
88
const uri = URI_STRING;
99

1010
const { MongoClient, ClientEncryption, BSON } = require(driverPath);
1111
const process = require('node:process');
1212
const util = require('node:util');
1313
const timers = require('node:timers');
1414
const fs = require('node:fs');
15-
const chai = require('chai');
15+
const { expect } = require('chai');
1616
const { setTimeout } = require('timers');
1717

1818
let originalReport;
1919
const logFile = 'logs.txt';
2020

2121
const run = func;
22-
const serverType = ['tcp', 'udp'];
2322

24-
// Returns an array containing new the resources created after script start
23+
/**
24+
*
25+
* Returns an array containing the new resources created after script started.
26+
* A new resource is something that will keep the event loop running.
27+
*
28+
* In order to be counted as a new resource, a resource MUST:
29+
* - Must NOT share an address with a libuv resource that existed at the start of script
30+
* - Must be referenced. See [here](https://nodejs.org/api/timers.html#timeoutref) for more context.
31+
* - Must NOT be an inactive server
32+
*
33+
* We're using the following tool to track resources: `process.report.getReport().libuv`
34+
* For more context, see documentation for [process.report.getReport()](https://nodejs.org/api/report.html), and [libuv](https://docs.libuv.org/en/v1.x/handle.html).
35+
*
36+
*/
2537
function getNewLibuvResourceArray() {
2638
let currReport = process.report.getReport().libuv;
2739
const originalReportAddresses = originalReport.map(resource => resource.address);
28-
currReport = currReport.filter(
29-
resource =>
40+
41+
/**
42+
* @typedef {Object} LibuvResource
43+
* @property {boolean} is_active Is the resource active? For a socket, this means it is allowing I/O. For a timer, this means a timer is has not expired.
44+
* @property {string} type What is the resource type? For example, 'tcp' | 'timer' | 'udp' | 'tty'... (See more in [docs](https://docs.libuv.org/en/v1.x/handle.html)).
45+
* @property {boolean} is_referenced Is the resource keeping the JS event loop active?
46+
*
47+
* @param {LibuvResource} resource
48+
*/
49+
function isNewLibuvResource(resource) {
50+
const serverType = ['tcp', 'udp'];
51+
return (
3052
!originalReportAddresses.includes(resource.address) &&
3153
resource.is_referenced && // if a resource is unreferenced, it's not keeping the event loop open
3254
(!serverType.includes(resource.type) || resource.is_active)
33-
);
55+
);
56+
}
57+
58+
currReport = currReport.filter(resource => isNewLibuvResource(resource));
3459
return currReport;
3560
}
3661

62+
// A log function for debugging
3763
function log(message) {
3864
// remove outer parentheses for easier parsing
3965
const messageToLog = JSON.stringify(message) + ' \n';
@@ -45,22 +71,19 @@ async function main() {
4571
process.on('beforeExit', () => {
4672
log({ beforeExitHappened: true });
4773
});
48-
await run({ MongoClient, uri, log, chai, ClientEncryption, BSON });
74+
await run({ MongoClient, uri, log, expect, ClientEncryption, BSON });
4975
log({ newLibuvResources: getNewLibuvResourceArray() });
5076
}
5177

5278
main()
53-
.then(() => {
54-
log({ exitCode: 0 });
55-
})
79+
.then(() => {})
5680
.catch(e => {
5781
log({ exitCode: 1, error: util.inspect(e) });
5882
});
5983

6084
setTimeout(() => {
6185
// this means something was in the event loop such that it hung for more than 10 seconds
6286
// so we kill the process
63-
log({ exitCode: 99 });
6487
process.exit(99);
6588
// using `unref` will ensure this setTimeout call is not a resource / does not keep the event loop running
6689
}, 10000).unref();

0 commit comments

Comments
 (0)