Skip to content

Commit fa4b01b

Browse files
committed
fix(server): non-timeout network errors transition to Unknown state
Per the SDAM rules in the error handling section, a non-timeout error encountered during operation execution MUST transition the server to an `Unknown` state and kick off the SDAM flow. NODE-2435
1 parent 3efa4c7 commit fa4b01b

File tree

4 files changed

+48
-3
lines changed

4 files changed

+48
-3
lines changed

lib/core/error.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,10 @@ function isSDAMUnrecoverableError(error) {
251251
return false;
252252
}
253253

254+
function isNetworkTimeoutError(err) {
255+
return err instanceof MongoNetworkError && err.message.match(/timed out/);
256+
}
257+
254258
module.exports = {
255259
MongoError,
256260
MongoNetworkError,
@@ -261,5 +265,6 @@ module.exports = {
261265
mongoErrorContextSymbol,
262266
isRetryableError,
263267
isSDAMUnrecoverableError,
264-
isNodeShuttingDownError
268+
isNodeShuttingDownError,
269+
isNetworkTimeoutError
265270
};

lib/core/sdam/server.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const MongoNetworkError = require('../error').MongoNetworkError;
1313
const collationNotSupported = require('../utils').collationNotSupported;
1414
const debugOptions = require('../connection/utils').debugOptions;
1515
const isSDAMUnrecoverableError = require('../error').isSDAMUnrecoverableError;
16+
const isNetworkTimeoutError = require('../error').isNetworkTimeoutError;
1617
const makeStateMachine = require('../utils').makeStateMachine;
1718
const common = require('./common');
1819

@@ -447,9 +448,11 @@ function makeOperationHandler(server, options, callback) {
447448
if (options && options.session) {
448449
options.session.serverSession.isDirty = true;
449450
}
450-
}
451451

452-
if (isSDAMUnrecoverableError(err)) {
452+
if (!isNetworkTimeoutError(err)) {
453+
server.emit('error', err);
454+
}
455+
} else if (isSDAMUnrecoverableError(err)) {
453456
server.emit('error', err);
454457
}
455458
}

lib/core/sdam/topology.js

+3
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,9 @@ function destroyServer(server, topology, options, callback) {
795795
options = options || {};
796796
LOCAL_SERVER_EVENTS.forEach(event => server.removeAllListeners(event));
797797

798+
// register a no-op for errors, we don't care now that we are destroying the server
799+
server.on('error', () => {});
800+
798801
server.destroy(options, () => {
799802
topology.emit(
800803
'serverClosed',

test/unit/sdam/topology.test.js

+34
Original file line numberDiff line numberDiff line change
@@ -192,5 +192,39 @@ describe('Topology (unit)', function() {
192192
});
193193
});
194194
});
195+
196+
it('should set server to unknown on non-timeout network error', function(done) {
197+
mockServer.setMessageHandler(request => {
198+
const doc = request.document;
199+
if (doc.ismaster) {
200+
request.reply(Object.assign({}, mock.DEFAULT_ISMASTER, { maxWireVersion: 9 }));
201+
} else if (doc.insert) {
202+
request.connection.destroy();
203+
} else {
204+
request.reply({ ok: 1 });
205+
}
206+
});
207+
208+
const topology = new Topology(mockServer.uri());
209+
topology.connect(err => {
210+
expect(err).to.not.exist;
211+
212+
topology.selectServer('primary', (err, server) => {
213+
expect(err).to.not.exist;
214+
this.defer(() => topology.close());
215+
216+
let serverError;
217+
server.on('error', err => (serverError = err));
218+
219+
server.command('test.test', { insert: { a: 42 } }, (err, result) => {
220+
expect(result).to.not.exist;
221+
expect(err).to.exist;
222+
expect(err).to.eql(serverError);
223+
expect(server.description.type).to.equal('Unknown');
224+
done();
225+
});
226+
});
227+
});
228+
});
195229
});
196230
});

0 commit comments

Comments
 (0)