Skip to content

Commit 5fd5b4d

Browse files
authored
Merge pull request #15229 from Automattic/vkarpov15/gh-15201
feat(connection): make connection helpers respect bufferTimeoutMS
2 parents 0722e8f + 5fad5a3 commit 5fd5b4d

File tree

6 files changed

+92
-16
lines changed

6 files changed

+92
-16
lines changed

lib/collection.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -311,13 +311,7 @@ Collection.prototype._getBufferTimeoutMS = function _getBufferTimeoutMS() {
311311
if (opts && opts.schemaUserProvidedOptions != null && opts.schemaUserProvidedOptions.bufferTimeoutMS != null) {
312312
return opts.schemaUserProvidedOptions.bufferTimeoutMS;
313313
}
314-
if (conn.config.bufferTimeoutMS != null) {
315-
return conn.config.bufferTimeoutMS;
316-
}
317-
if (conn.base != null && conn.base.get('bufferTimeoutMS') != null) {
318-
return conn.base.get('bufferTimeoutMS');
319-
}
320-
return 10000;
314+
return conn._getBufferTimeoutMS();
321315
};
322316

323317
/*!

lib/connection.js

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -824,10 +824,54 @@ Connection.prototype.dropCollection = async function dropCollection(collection)
824824

825825
Connection.prototype._waitForConnect = async function _waitForConnect() {
826826
if ((this.readyState === STATES.connecting || this.readyState === STATES.disconnected) && this._shouldBufferCommands()) {
827-
await new Promise(resolve => {
828-
this._queue.push({ fn: resolve });
829-
});
827+
const bufferTimeoutMS = this._getBufferTimeoutMS();
828+
let timeout = null;
829+
let timedOut = false;
830+
// The element that this function pushes onto `_queue`, stored to make it easy to remove later
831+
const queueElement = {};
832+
await Promise.race([
833+
new Promise(resolve => {
834+
queueElement.fn = resolve;
835+
this._queue.push(queueElement);
836+
}),
837+
new Promise(resolve => {
838+
timeout = setTimeout(
839+
() => {
840+
timedOut = true;
841+
resolve();
842+
},
843+
bufferTimeoutMS
844+
);
845+
})
846+
]);
847+
848+
if (timedOut) {
849+
const index = this._queue.indexOf(queueElement);
850+
if (index !== -1) {
851+
this._queue.splice(index, 1);
852+
}
853+
const message = 'Connection operation buffering timed out after ' + bufferTimeoutMS + 'ms';
854+
throw new MongooseError(message);
855+
} else if (timeout != null) {
856+
// Not strictly necessary, but avoid the extra overhead of creating a new MongooseError
857+
// in case of success
858+
clearTimeout(timeout);
859+
}
860+
}
861+
};
862+
863+
/*!
864+
* Get the default buffer timeout for this connection
865+
*/
866+
867+
Connection.prototype._getBufferTimeoutMS = function _getBufferTimeoutMS() {
868+
if (this.config.bufferTimeoutMS != null) {
869+
return this.config.bufferTimeoutMS;
830870
}
871+
if (this.base != null && this.base.get('bufferTimeoutMS') != null) {
872+
return this.base.get('bufferTimeoutMS');
873+
}
874+
return 10000;
831875
};
832876

833877
/**
@@ -1156,6 +1200,10 @@ Connection.prototype.close = async function close(force) {
11561200
this.$wasForceClosed = !!force;
11571201
}
11581202

1203+
if (this._lastHeartbeatAt != null) {
1204+
this._lastHeartbeatAt = null;
1205+
}
1206+
11591207
for (const model of Object.values(this.models)) {
11601208
// If manually disconnecting, make sure to clear each model's `$init`
11611209
// promise, so Mongoose knows to re-run `init()` in case the

lib/drivers/node-mongodb-native/connection.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,11 @@ NativeConnection.prototype.createClient = async function createClient(uri, optio
281281
delete options.autoSearchIndex;
282282
}
283283

284+
if ('bufferTimeoutMS' in options) {
285+
this.config.bufferTimeoutMS = options.bufferTimeoutMS;
286+
delete options.bufferTimeoutMS;
287+
}
288+
284289
// Backwards compat
285290
if (options.user || options.pass) {
286291
options.auth = options.auth || {};
@@ -426,6 +431,9 @@ function _setClient(conn, client, options, dbName) {
426431
}
427432
});
428433
}
434+
435+
conn._lastHeartbeatAt = null;
436+
429437
client.on('serverHeartbeatSucceeded', () => {
430438
conn._lastHeartbeatAt = Date.now();
431439
});

lib/model.js

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ const prepareDiscriminatorPipeline = require('./helpers/aggregate/prepareDiscrim
6464
const pushNestedArrayPaths = require('./helpers/model/pushNestedArrayPaths');
6565
const removeDeselectedForeignField = require('./helpers/populate/removeDeselectedForeignField');
6666
const setDottedPath = require('./helpers/path/setDottedPath');
67-
const STATES = require('./connectionState');
6867
const util = require('util');
6968
const utils = require('./utils');
7069
const minimize = require('./helpers/minimize');
@@ -1104,11 +1103,7 @@ Model.init = function init() {
11041103
return results;
11051104
};
11061105
const _createCollection = async() => {
1107-
if ((conn.readyState === STATES.connecting || conn.readyState === STATES.disconnected) && conn._shouldBufferCommands()) {
1108-
await new Promise(resolve => {
1109-
conn._queue.push({ fn: resolve });
1110-
});
1111-
}
1106+
await conn._waitForConnect();
11121107
const autoCreate = utils.getOption(
11131108
'autoCreate',
11141109
this.schema.options,

test/collection.test.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,16 @@ describe('collections:', function() {
6464
});
6565
});
6666

67+
it('handles bufferTimeoutMS in schemaUserProvidedOptions', async function() {
68+
db = mongoose.createConnection();
69+
const collection = db.collection('gh14184');
70+
collection.opts.schemaUserProvidedOptions = { bufferTimeoutMS: 100 };
71+
72+
const err = await collection.find({ foo: 'bar' }, {}).then(() => null, err => err);
73+
assert.ok(err);
74+
assert.ok(err.message.includes('buffering timed out after 100ms'));
75+
});
76+
6777
it('methods should that throw (unimplemented)', function() {
6878
const collection = new Collection('test', mongoose.connection);
6979
let thrown = false;

test/connection.test.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,6 +1787,27 @@ describe('connections:', function() {
17871787
assert.ok(res.mongoose.results[1].message.includes('not a number'));
17881788
});
17891789

1790+
it('buffers connection helpers', async function() {
1791+
const m = new mongoose.Mongoose();
1792+
1793+
const promise = m.connection.listCollections();
1794+
1795+
await new Promise(resolve => setTimeout(resolve, 100));
1796+
await m.connect(start.uri, { bufferTimeoutMS: 1000 });
1797+
await promise;
1798+
1799+
await m.connection.listCollections();
1800+
1801+
await m.disconnect();
1802+
});
1803+
1804+
it('connection helpers buffering times out', async function() {
1805+
const m = new mongoose.Mongoose();
1806+
m.set('bufferTimeoutMS', 100);
1807+
1808+
await assert.rejects(m.connection.listCollections(), /Connection operation buffering timed out after 100ms/);
1809+
});
1810+
17901811
it('supports db-level aggregate on connection (gh-15118)', async function() {
17911812
const db = start();
17921813

0 commit comments

Comments
 (0)