Skip to content

Commit 6e3bab3

Browse files
authored
fix(cmap): undo flipping of beforeHandshake flag for timeout errors (#2813)
1 parent 4fd03e8 commit 6e3bab3

File tree

4 files changed

+83
-3
lines changed

4 files changed

+83
-3
lines changed

lib/cmap/connection.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ class Connection extends EventEmitter {
122122
if (issue.isTimeout) {
123123
op.cb(
124124
new MongoNetworkTimeoutError(`connection ${this.id} to ${this.address} timed out`, {
125-
beforeHandshake: !!this[kIsMaster]
125+
beforeHandshake: this.ismaster == null
126126
})
127127
);
128128
} else if (issue.isClose) {

lib/core/error.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ class MongoNetworkError extends MongoError {
102102
super(message);
103103
this.name = 'MongoNetworkError';
104104

105-
if (options && options.beforeHandshake === true) {
106-
this[kBeforeHandshake] = true;
105+
if (options && typeof options.beforeHandshake === 'boolean') {
106+
this[kBeforeHandshake] = options.beforeHandshake;
107107
}
108108
}
109109
}

test/unit/cmap/connection.test.js

+58
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,62 @@ describe('Connection', function() {
6666
}
6767
);
6868
});
69+
70+
it('should throw a network error with kBeforeHandshake set to false on timeout after hand shake', function(done) {
71+
server.setMessageHandler(request => {
72+
const doc = request.document;
73+
if (doc.ismaster) {
74+
request.reply(mock.DEFAULT_ISMASTER_36);
75+
}
76+
// respond to no other requests to trigger timeout event
77+
});
78+
79+
const address = server.address();
80+
const options = {
81+
bson: new BSON(),
82+
connectionType: Connection,
83+
host: address.host,
84+
port: address.port
85+
};
86+
87+
connect(options, (err, conn) => {
88+
expect(err).to.be.a('undefined');
89+
expect(conn).to.be.instanceOf(Connection);
90+
expect(conn)
91+
.to.have.property('ismaster')
92+
.that.is.a('object');
93+
94+
conn.command('$admin.cmd', { ping: 1 }, { socketTimeout: 50 }, err => {
95+
const beforeHandshakeSymbol = Object.getOwnPropertySymbols(err)[0];
96+
expect(beforeHandshakeSymbol).to.be.a('symbol');
97+
expect(err).to.have.property(beforeHandshakeSymbol, false);
98+
99+
done();
100+
});
101+
});
102+
});
103+
104+
it('should throw a network error with kBeforeHandshake set to true on timeout before hand shake', function(done) {
105+
// respond to no requests to trigger timeout event
106+
server.setMessageHandler(() => {});
107+
108+
const address = server.address();
109+
const options = {
110+
bson: new BSON(),
111+
connectionType: Connection,
112+
host: address.host,
113+
port: address.port,
114+
socketTimeout: 50
115+
};
116+
117+
connect(options, (err, conn) => {
118+
expect(conn).to.be.a('undefined');
119+
120+
const beforeHandshakeSymbol = Object.getOwnPropertySymbols(err)[0];
121+
expect(beforeHandshakeSymbol).to.be.a('symbol');
122+
expect(err).to.have.property(beforeHandshakeSymbol, true);
123+
124+
done();
125+
});
126+
});
69127
});

test/unit/error.test.js

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
'use strict';
2+
3+
const expect = require('chai').expect;
4+
const MongoNetworkError = require('../../lib/core/error').MongoNetworkError;
5+
6+
describe('MongoErrors', function() {
7+
describe('MongoNetworkError', function() {
8+
it('should only define beforeHandshake symbol if boolean option passed in', function() {
9+
const errorWithOptionTrue = new MongoNetworkError('', { beforeHandshake: true });
10+
expect(Object.getOwnPropertySymbols(errorWithOptionTrue).length).to.equal(1);
11+
12+
const errorWithOptionFalse = new MongoNetworkError('', { beforeHandshake: false });
13+
expect(Object.getOwnPropertySymbols(errorWithOptionFalse).length).to.equal(1);
14+
15+
const errorWithBadOption = new MongoNetworkError('', { beforeHandshake: 'not boolean' });
16+
expect(Object.getOwnPropertySymbols(errorWithBadOption).length).to.equal(0);
17+
18+
const errorWithoutOption = new MongoNetworkError('');
19+
expect(Object.getOwnPropertySymbols(errorWithoutOption).length).to.equal(0);
20+
});
21+
});
22+
});

0 commit comments

Comments
 (0)