Skip to content

fix(NODE-5925): driver throws error when non-read operation in a transaction has a ReadPreferenceMode other than 'primary' #4075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 15, 2024
13 changes: 12 additions & 1 deletion src/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
ByteUtils,
calculateDurationInMs,
type Callback,
commandIsReadOperation,
commandSupportsReadConcern,
isPromiseLike,
List,
Expand Down Expand Up @@ -194,7 +195,7 @@ export class ClientSession extends TypedEventEmitter<ClientSessionEvents> {

this.operationTime = undefined;
this.owner = options.owner;
this.defaultTransactionOptions = Object.assign({}, options.defaultTransactionOptions);
this.defaultTransactionOptions = { ...options.defaultTransactionOptions };
this.transaction = new Transaction();
}

Expand Down Expand Up @@ -1039,6 +1040,16 @@ export function applySession(
command.readConcern = readConcern;
}

if (
commandIsReadOperation(command) &&
((typeof session.transaction.options.readPreference === 'string' &&
session.transaction.options.readPreference !== 'primary') ||
(typeof session.transaction.options.readPreference === 'object' &&
session.transaction.options.readPreference.mode !== 'primary'))
) {
throw new MongoTransactionError('read preference in a transaction must be primary');
}

if (session.supports.causalConsistency && session.operationTime) {
command.readConcern = command.readConcern || {};
Object.assign(command.readConcern, { afterClusterTime: session.operationTime });
Expand Down
14 changes: 14 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1077,6 +1077,20 @@ export function commandSupportsReadConcern(command: Document): boolean {
return false;
}

export function commandIsReadOperation(command: Document): boolean {
if (
command.find ||
command.findOne ||
command.ensureIndex ||
command.count ||
command.aggregate ||
command.distinct
) {
return true;
}
return false;
}

/**
* Compare objectIds. `null` is always less
* - `+1 = oid1 is greater than oid2`
Expand Down
4 changes: 1 addition & 3 deletions test/integration/transactions/transactions.spec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import { loadSpecTests } from '../../spec';
import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';

const SKIPPED_TESTS = [
// TODO(NODE-5925) - secondary read preference not allowed in transactions.
'readPreference inherited from defaultTransactionOptions',
// TODO(NODE-5924) - Fix modification of readConcern object post message send.
'readConcern local in defaultTransactionOptions',
'defaultTransactionOptions override client options',
Expand All @@ -17,7 +15,7 @@ const SKIPPED_TESTS = [
describe('Transactions Spec Unified Tests', function () {
runUnifiedSuite(loadSpecTests(path.join('transactions', 'unified')), test => {
return SKIPPED_TESTS.includes(test.description)
? 'TODO(NODE-5924/NODE-5925): Skipping failing transaction tests'
? 'TODO(NODE-5924): Skipping failing transaction tests'
: false;
});
});
5 changes: 2 additions & 3 deletions test/tools/unified-spec-runner/entities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -620,9 +620,8 @@ export class EntitiesMap<E = Entity> extends Map<string, E> {
WriteConcern.fromOptions(defaultOptions);
}
if (defaultOptions.readPreference) {
options.defaultTransactionOptions.readPreference = ReadPreference.fromOptions(
defaultOptions.readPreference
);
options.defaultTransactionOptions.readPreference =
ReadPreference.fromOptions(defaultOptions);
}
if (typeof defaultOptions.maxCommitTimeMS === 'number') {
options.defaultTransactionOptions.maxCommitTimeMS = defaultOptions.maxCommitTimeMS;
Expand Down
Loading