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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
082a3be
fix(NODE-5925): Attempting a read on txn with a non-primary readPrefe…
aditi-khare-mongoDB Apr 11, 2024
2138e47
updated commandIsReadOperation to be inspired by READ_OPERATION aspect
aditi-khare-mongoDB Apr 11, 2024
fe5cfb2
removed extraneous code and started using aspect.READ_OPERATION direc…
aditi-khare-mongoDB Apr 11, 2024
8389a35
Merge branch 'main' into NODE-5925/no-secondary-readpref-on-txn
aditi-khare-mongoDB Apr 11, 2024
4b4ccdf
throw error on runCommand too
aditi-khare-mongoDB Apr 11, 2024
22d73a9
Merge branch 'main' into NODE-5925/no-secondary-readpref-on-txn
aditi-khare-mongoDB Apr 11, 2024
565b193
pr requested changes
aditi-khare-mongoDB Apr 15, 2024
96d07c2
Merge branch 'main' into NODE-5925/no-secondary-readpref-on-txn
aditi-khare-mongoDB Apr 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/operations/execute_operation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,14 @@ export async function executeOperation<
const readPreference = operation.readPreference ?? ReadPreference.primary;
const inTransaction = !!session?.inTransaction();

if (inTransaction && !readPreference.equals(ReadPreference.primary)) {
const hasReadAspect = operation.hasAspect(Aspect.READ_OPERATION);
const hasWriteAspect = operation.hasAspect(Aspect.WRITE_OPERATION);

if (
inTransaction &&
!readPreference.equals(ReadPreference.primary) &&
(hasReadAspect || operation.commandName === 'runCommand')
) {
throw new MongoTransactionError(
`Read preference in a transaction must be primary, not: ${readPreference.mode}`
);
Expand Down Expand Up @@ -177,8 +184,6 @@ export async function executeOperation<
supportsRetryableWrites(server) &&
operation.canRetryWrite;

const hasReadAspect = operation.hasAspect(Aspect.READ_OPERATION);
const hasWriteAspect = operation.hasAspect(Aspect.WRITE_OPERATION);
const willRetry = (hasReadAspect && willRetryRead) || (hasWriteAspect && willRetryWrite);

if (hasWriteAspect && willRetryWrite) {
Expand Down
2 changes: 1 addition & 1 deletion src/sessions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,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
4 changes: 1 addition & 3 deletions test/integration/transactions/transactions.spec.test.ts
View file
Open in desktop
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
View file
Open in desktop
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