Skip to content

Commit

Permalink
fix: move session support check to operation layer
Browse files Browse the repository at this point in the history
Monitor checks run on a timer and network errors cause a reset
of the topology that would be corrected in the next cycle of the
monitor, we were checking a property that gets updated by this async
work. By moving the check to the operations layer we will allow users
to obtain a session regardless of support and then emit an error if
there is not support when the session is used in an operation.

NODE-3100
  • Loading branch information
nbbeeken committed Feb 16, 2021
1 parent d67ffa7 commit 7909c78
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 18 deletions.
4 changes: 0 additions & 4 deletions lib/mongo_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -457,10 +457,6 @@ MongoClient.prototype.startSession = function(options) {
throw new MongoError('Must connect to a server before calling this method');
}

if (!this.topology.hasSessionSupport()) {
throw new MongoError('Current topology does not support sessions');
}

return this.topology.startSession(options, this.s.options);
};

Expand Down
2 changes: 2 additions & 0 deletions lib/operations/execute_operation.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ function executeOperation(topology, operation, callback) {
} else if (operation.session.hasEnded) {
throw new MongoError('Use of expired sessions is not permitted');
}
} else if (operation.session && operation.session.explicit) {
throw new MongoError('Current topology does not support sessions');
}

let result;
Expand Down
48 changes: 34 additions & 14 deletions test/unit/sessions/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('Sessions', function() {

it('should throw an exception if sessions are not supported', {
metadata: { requires: { topology: 'single' } },
test: function(done) {
test() {
test.server.setMessageHandler(request => {
var doc = request.document;
if (doc.ismaster) {
Expand All @@ -27,21 +27,32 @@ describe('Sessions', function() {
});

const client = this.configuration.newClient(`mongodb://${test.server.uri()}/test`);
client.connect(function(err, client) {
expect(err).to.not.exist;
expect(() => {
client.startSession();
}).to.throw(/Current topology does not support sessions/);

client.close(done);
});
return client
.connect()
.then(function(client) {
const session = client.startSession();
return client
.db()
.collection('t')
.insertOne({ a: 1 }, { session });
})
.then(() => {
expect.fail('Expected an error to be thrown about not supporting sessions');
})
.catch(error => {
expect(error.message).to.equal('Current topology does not support sessions');
})
.then(() => {
return client.close();
});
}
});

it('should throw an exception if sessions are not supported on some servers', {
metadata: { requires: { topology: 'single' } },
test() {
const replicaSetMock = new ReplSetFixture();
let client;
return replicaSetMock
.setup({ doNotInitHandlers: true })
.then(() => {
Expand Down Expand Up @@ -92,14 +103,23 @@ describe('Sessions', function() {
return replicaSetMock.uri();
})
.then(uri => {
const client = this.configuration.newClient(uri);
client = this.configuration.newClient(uri);
return client.connect();
})
.then(client => {
expect(client.topology.s.description.logicalSessionTimeoutMinutes).to.not.exist;
expect(() => {
client.startSession();
}).to.throw(/Current topology does not support sessions/);
const session = client.startSession();
return client
.db()
.collection('t')
.insertOne({ a: 1 }, { session });
})
.then(() => {
expect.fail('Expected an error to be thrown about not supporting sessions');
})
.catch(error => {
expect(error.message).to.equal('Current topology does not support sessions');
})
.then(() => {
return client.close();
});
}
Expand Down

0 comments on commit 7909c78

Please sign in to comment.