Skip to content

Commit

Permalink
fix: handle session with unacknowledged write
Browse files Browse the repository at this point in the history
If there's an explicit session and the writeConcern is `unacknowledged` an error will now be thrown.  If there's an implicit session and the writeConcern is `unacknowledged` the session will not be sent to the server.

NODE-1341
  • Loading branch information
Thomas Reggi authored Aug 14, 2020
1 parent bb13764 commit 4aeaedf
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 2 deletions.
8 changes: 8 additions & 0 deletions src/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,14 @@ function applySession(session: any, command: any, options?: any): MongoError | u
return new MongoError('Cannot use a session that has ended');
}

// SPEC-1019: silently ignore explicit session with unacknowledged write for backwards compatibility
if (options && options.writeConcern && options.writeConcern.w === 0) {
if (session && session.explicit) {
return new MongoError('Cannot have explicit session with unacknowledged writes');
}
return;
}

// mark the last use of this session, and apply the `lsid`
serverSession.lastUse = now();
command.lsid = serverSession.id;
Expand Down
46 changes: 44 additions & 2 deletions test/functional/sessions.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
'use strict';

const expect = require('chai').expect;
const setupDatabase = require('./shared').setupDatabase;
const withMonitoredClient = require('./shared').withMonitoredClient;
const TestRunnerContext = require('./spec-runner').TestRunnerContext;
const generateTopologyTests = require('./spec-runner').generateTopologyTests;
const loadSpecTests = require('../spec').loadSpecTests;
Expand Down Expand Up @@ -43,7 +45,7 @@ describe('Sessions', function () {

it('should send endSessions for multiple sessions', {
metadata: {
requires: { topology: ['single'], mongodb: '>3.6.0' },
requires: { topology: ['single'], mongodb: '>=3.6.0' },
// Skipping session leak tests b/c these are explicit sessions
sessions: { skipLeakTests: true }
},
Expand All @@ -65,7 +67,7 @@ describe('Sessions', function () {
});

describe('withSession', {
metadata: { requires: { mongodb: '>3.6.0' } },
metadata: { requires: { mongodb: '>=3.6.0' } },
test: function () {
beforeEach(function () {
return test.setup(this.configuration);
Expand Down Expand Up @@ -194,4 +196,44 @@ describe('Sessions', function () {

generateTopologyTests(testSuites, testContext, testFilter);
});

context('unacknowledged writes', () => {
it('should not include session for unacknowledged writes', {
metadata: { requires: { topology: 'single', mongodb: '>=3.6.0' } },
test: withMonitoredClient('insert', { clientOptions: { w: 0 } }, function (
client,
events,
done
) {
client
.db('test')
.collection('foo')
.insertOne({ foo: 'bar' }, err => {
expect(err).to.not.exist;
const event = events[0];
expect(event).nested.property('command.writeConcern.w').to.equal(0);
expect(event).to.not.have.nested.property('command.lsid');
done();
});
})
});
it('should throw error with explicit session', {
metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6.0' } },
test: withMonitoredClient('insert', { clientOptions: { w: 0 } }, function (
client,
events,
done
) {
const session = client.startSession({ causalConsistency: true });
client
.db('test')
.collection('foo')
.insertOne({ foo: 'bar' }, { session }, err => {
expect(err).to.exist;
expect(err.message).to.equal('Cannot have explicit session with unacknowledged writes');
client.close(done);
});
})
});
});
});

0 comments on commit 4aeaedf

Please sign in to comment.