Skip to content

Commit aee8f57

Browse files
committed
refactor: improve error message for MongoTimeoutErrors
We will now surface the first encountered error in the set of known server descriptions as the primary error message for a timeout error. The `reason` field for server selection errors will now include the `TopologyDescription` for better traceability NODE-2397
1 parent 8c89b89 commit aee8f57

File tree

5 files changed

+14
-16
lines changed

5 files changed

+14
-16
lines changed

lib/core/error.js

+7-2
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,14 @@ class MongoParseError extends MongoError {
9595
*/
9696
class MongoTimeoutError extends MongoError {
9797
constructor(message, reason) {
98-
super(message);
98+
if (reason && reason.error) {
99+
super(reason.error);
100+
} else {
101+
super(message);
102+
}
103+
99104
this.name = 'MongoTimeoutError';
100-
if (reason != null) {
105+
if (reason) {
101106
this.reason = reason;
102107
}
103108
}

lib/core/sdam/server_selection.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ function selectServers(topology, selector, timeout, start, callback) {
261261
if (duration >= timeout) {
262262
return callback(
263263
new MongoTimeoutError(`Server selection timed out after ${timeout} ms`),
264-
topology.description.error
264+
topology.description
265265
);
266266
}
267267

@@ -308,7 +308,7 @@ function selectServers(topology, selector, timeout, start, callback) {
308308
callback(
309309
new MongoTimeoutError(
310310
`Server selection timed out after ${timeout} ms`,
311-
topology.description.error
311+
topology.description
312312
)
313313
);
314314
}, timeout - duration);

test/functional/core/server.test.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -1002,8 +1002,7 @@ describe('Server tests', function() {
10021002
let err;
10031003
try {
10041004
expect(error).to.be.an.instanceOf(Error);
1005-
const errorMessage = error.reason ? error.reason.message : error.message;
1006-
expect(errorMessage).to.match(/but this version of the Node.js Driver requires/);
1005+
expect(error).to.match(/but this version of the Node.js Driver requires/);
10071006
} catch (e) {
10081007
err = e;
10091008
}

test/functional/scram_sha_256.test.js

+2-8
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,7 @@ describe('SCRAM-SHA-256 auth', function() {
186186
return withClient(
187187
this.configuration.newClient({}, options),
188188
() => Promise.reject(new Error('This request should have failed to authenticate')),
189-
err => {
190-
const errMessage = err.reason ? err.reason.message : err;
191-
expect(errMessage).to.match(/Authentication failed/);
192-
}
189+
err => expect(err).to.match(/Authentication failed/)
193190
);
194191
}
195192
});
@@ -223,10 +220,7 @@ describe('SCRAM-SHA-256 auth', function() {
223220
withClient(
224221
this.configuration.newClient({}, options),
225222
() => Promise.reject(new Error('This request should have failed to authenticate')),
226-
err => {
227-
const errMessage = err.reason ? err.reason.message : err;
228-
expect(errMessage).to.match(/Authentication failed/);
229-
}
223+
err => expect(err).to.match(/Authentication failed/)
230224
);
231225

232226
return Promise.all([getErrorMsg(noUsernameOptions), getErrorMsg(badPasswordOptions)]);

test/unit/sdam/server_selection/select_servers.test.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe('selectServers', function() {
3535
selectServers(topology, ReadPreference.primary, 500, process.hrtime(), err => {
3636
expect(err).to.exist;
3737
expect(err).to.match(/Server selection timed out/);
38-
expect(err).to.not.have.property('reason');
38+
expect(err).to.have.property('reason');
3939

4040
done();
4141
});
@@ -60,7 +60,7 @@ describe('selectServers', function() {
6060
selectServers(topology, ReadPreference.primary, 1000, process.hrtime(), err => {
6161
expect(err).to.exist;
6262
expect(err).to.match(/Server selection timed out/);
63-
expect(err).to.not.have.property('reason');
63+
expect(err).to.have.property('reason');
6464

6565
// expect a call to monitor for initial server creation, and another for the server selection
6666
expect(serverMonitor)

0 commit comments

Comments
 (0)