Skip to content

Commit b1cb840

Browse files
tadjik1tadjik1
committed
fix(NODE-7430): throw timeout error when withTransaction retries exceed deadline
1 parent 275afa5 commit b1cb840

4 files changed

Lines changed: 399 additions & 46 deletions

File tree

‎src/sessions.ts‎

Lines changed: 62 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
MongoErrorLabel,
1818
MongoExpiredSessionError,
1919
MongoInvalidArgumentError,
20+
MongoOperationTimeoutError,
2021
MongoRuntimeError,
2122
MongoServerError,
2223
MongoTransactionError,
@@ -777,14 +778,15 @@ export class ClientSession
777778
const willExceedTransactionDeadline =
778779
(this.timeoutContext?.csotEnabled() &&
779780
backoffMS > this.timeoutContext.remainingTimeMS) ||
780-
processTimeMS() + backoffMS > startTime + MAX_TIMEOUT;
781+
(!this.timeoutContext?.csotEnabled() &&
782+
processTimeMS() + backoffMS > startTime + MAX_TIMEOUT);
781783

782784
if (willExceedTransactionDeadline) {
783-
throw (
785+
throw makeWithTransactionTimeoutError(
784786
lastError ??
785-
new MongoRuntimeError(
786-
`Transaction retry did not record an error: should never occur. Please file a bug.`
787-
)
787+
new MongoRuntimeError(
788+
`Transaction retry did not record an error: should never occur. Please file a bug.`
789+
)
788790
);
789791
}
790792

@@ -827,6 +829,8 @@ export class ClientSession
827829
throw fnError;
828830
}
829831

832+
lastError = fnError;
833+
830834
if (
831835
this.transaction.state === TxnState.STARTING_TRANSACTION ||
832836
this.transaction.state === TxnState.TRANSACTION_IN_PROGRESS
@@ -836,14 +840,15 @@ export class ClientSession
836840
await this.abortTransaction();
837841
}
838842

839-
if (
840-
fnError.hasErrorLabel(MongoErrorLabel.TransientTransactionError) &&
841-
(this.timeoutContext?.csotEnabled() || processTimeMS() - startTime < MAX_TIMEOUT)
842-
) {
843-
// 7.ii If the callback's error includes a "TransientTransactionError" label and the elapsed time of `withTransaction`
844-
// is less than 120 seconds, jump back to step two.
845-
lastError = fnError;
846-
continue retryTransaction;
843+
if (fnError.hasErrorLabel(MongoErrorLabel.TransientTransactionError)) {
844+
if (this.timeoutContext?.csotEnabled() || processTimeMS() - startTime < MAX_TIMEOUT) {
845+
// 7.ii If the callback's error includes a "TransientTransactionError" label and the elapsed time of `withTransaction`
846+
// is less than TIMEOUT_MS, jump back to step two.
847+
continue retryTransaction;
848+
} else {
849+
// 7.ii (cont.) If timeout has been exceeded, raise a timeout error wrapping the transient error.
850+
throw makeWithTransactionTimeoutError(fnError);
851+
}
847852
}
848853

849854
// 7.iii If the callback's error includes a "UnknownTransactionCommitResult" label, the callback must have manually committed a transaction,
@@ -865,37 +870,39 @@ export class ClientSession
865870
committed = true;
866871
// 10. If commitTransaction reported an error:
867872
} catch (commitError) {
868-
// If CSOT is enabled, we repeatedly retry until timeoutMS expires. This is enforced by providing a
869-
// timeoutContext to each async API, which know how to cancel themselves (i.e., the next retry will
870-
// abort the withTransaction call).
871-
// If CSOT is not enabled, do we still have time remaining or have we timed out?
873+
lastError = commitError;
874+
875+
// Check if the withTransaction timeout has been exceeded.
876+
// With CSOT: check remaining time from the timeout context.
877+
// Without CSOT: check if we've exceeded the 120-second timeout.
872878
const hasTimedOut =
873-
!this.timeoutContext?.csotEnabled() && processTimeMS() - startTime >= MAX_TIMEOUT;
874-
875-
if (!hasTimedOut) {
876-
/*
877-
* Note: a maxTimeMS error will have the MaxTimeMSExpired
878-
* code (50) and can be reported as a top-level error or
879-
* inside writeConcernError, ex.
880-
* { ok:0, code: 50, codeName: 'MaxTimeMSExpired' }
881-
* { ok:1, writeConcernError: { code: 50, codeName: 'MaxTimeMSExpired' } }
882-
*/
883-
if (
884-
!isMaxTimeMSExpiredError(commitError) &&
885-
commitError.hasErrorLabel(MongoErrorLabel.UnknownTransactionCommitResult)
886-
) {
887-
// 10.i If the `commitTransaction` error includes a "UnknownTransactionCommitResult" label and the error is not
888-
// MaxTimeMSExpired and the elapsed time of `withTransaction` is less than 120 seconds, jump back to step eight.
889-
continue retryCommit;
890-
}
891-
892-
if (commitError.hasErrorLabel(MongoErrorLabel.TransientTransactionError)) {
893-
// 10.ii If the commitTransaction error includes a "TransientTransactionError" label
894-
// and the elapsed time of withTransaction is less than 120 seconds, jump back to step two.
895-
lastError = commitError;
896-
897-
continue retryTransaction;
898-
}
879+
(this.timeoutContext?.csotEnabled() && this.timeoutContext.remainingTimeMS <= 0) ||
880+
(!this.timeoutContext?.csotEnabled() && processTimeMS() - startTime >= MAX_TIMEOUT);
881+
882+
if (hasTimedOut) {
883+
throw makeWithTransactionTimeoutError(commitError);
884+
}
885+
886+
/*
887+
* Note: a maxTimeMS error will have the MaxTimeMSExpired
888+
* code (50) and can be reported as a top-level error or
889+
* inside writeConcernError, ex.
890+
* { ok:0, code: 50, codeName: 'MaxTimeMSExpired' }
891+
* { ok:1, writeConcernError: { code: 50, codeName: 'MaxTimeMSExpired' } }
892+
*/
893+
if (
894+
!isMaxTimeMSExpiredError(commitError) &&
895+
commitError.hasErrorLabel(MongoErrorLabel.UnknownTransactionCommitResult)
896+
) {
897+
// 10.i If the `commitTransaction` error includes a "UnknownTransactionCommitResult" label and the error is not
898+
// MaxTimeMSExpired and the elapsed time of `withTransaction` is less than TIMEOUT_MS, jump back to step eight.
899+
continue retryCommit;
900+
}
901+
902+
if (commitError.hasErrorLabel(MongoErrorLabel.TransientTransactionError)) {
903+
// 10.ii If the commitTransaction error includes a "TransientTransactionError" label
904+
// and the elapsed time of withTransaction is less than TIMEOUT_MS, jump back to step two.
905+
continue retryTransaction;
899906
}
900907

901908
// 10.iii Otherwise, propagate the commitTransaction error to the caller of withTransaction and return immediately.
@@ -912,6 +919,18 @@ export class ClientSession
912919
}
913920
}
914921

922+
function makeWithTransactionTimeoutError(cause: Error): MongoOperationTimeoutError {
923+
const timeoutError = new MongoOperationTimeoutError('Timed out during withTransaction', {
924+
cause
925+
});
926+
if (cause instanceof MongoError) {
927+
for (const label of cause.errorLabels) {
928+
timeoutError.addErrorLabel(label);
929+
}
930+
}
931+
return timeoutError;
932+
}
933+
915934
const NON_DETERMINISTIC_WRITE_CONCERN_ERRORS = new Set([
916935
'CannotSatisfyWriteConcern',
917936
'UnknownReplWriteConcern',

‎test/integration/transactions-convenient-api/transactions-convenient-api.prose.test.ts‎

Lines changed: 166 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,18 @@ import { expect } from 'chai';
22
import { test } from 'mocha';
33
import * as sinon from 'sinon';
44

5-
import { type ClientSession, type Collection, type MongoClient } from '../../mongodb';
6-
import { configureFailPoint, type FailCommandFailPoint, measureDuration } from '../../tools/utils';
5+
import {
6+
type ClientSession,
7+
type Collection,
8+
type MongoClient,
9+
MongoOperationTimeoutError
10+
} from '../../mongodb';
11+
import {
12+
clearFailPoint,
13+
configureFailPoint,
14+
type FailCommandFailPoint,
15+
measureDuration
16+
} from '../../tools/utils';
717

818
const failCommand: FailCommandFailPoint = {
919
configureFailPoint: 'failCommand',
@@ -85,3 +95,157 @@ describe('Retry Backoff is Enforced', function () {
8595
}
8696
);
8797
});
98+
99+
describe('Retry Timeout is Enforced', function () {
100+
// Drivers should test that withTransaction enforces a non-configurable timeout before retrying
101+
// both commits and entire transactions.
102+
//
103+
// Note: We use CSOT's timeoutMS to enforce a short timeout instead of blocking for the full
104+
// 120-second retry timeout, as recommended by the spec: "This might be done by internally
105+
// modifying the timeout value used by withTransaction with some private API or using a mock timer."
106+
//
107+
// The error SHOULD be propagated as a timeout error if the language allows to expose the
108+
// underlying error as a cause of a timeout error.
109+
110+
let client: MongoClient;
111+
let collection: Collection;
112+
113+
beforeEach(async function () {
114+
client = this.configuration.newClient({ timeoutMS: 100 });
115+
collection = client.db('foo').collection('bar');
116+
});
117+
118+
afterEach(async function () {
119+
await clearFailPoint(this.configuration);
120+
await client?.close();
121+
});
122+
123+
// Case 1: If the callback raises an error with the TransientTransactionError label and the retry
124+
// timeout has been exceeded, withTransaction should propagate the error (see Note 1) to its caller.
125+
test(
126+
'callback TransientTransactionError propagated as timeout error when retry timeout exceeded',
127+
{
128+
requires: {
129+
mongodb: '>=4.4',
130+
topology: '!single'
131+
}
132+
},
133+
async function () {
134+
// 1. Configure a failpoint that always fails insert with TransientTransactionError
135+
// and blocks for 25ms to consume timeout budget.
136+
await configureFailPoint(this.configuration, {
137+
configureFailPoint: 'failCommand',
138+
mode: 'alwaysOn',
139+
data: {
140+
failCommands: ['insert'],
141+
blockConnection: true,
142+
blockTimeMS: 25,
143+
errorCode: 24,
144+
errorLabels: ['TransientTransactionError']
145+
}
146+
});
147+
148+
// 2. Run withTransaction with a callback that performs an insert.
149+
// The insert will always fail with TransientTransactionError, triggering retries
150+
// until the timeout (timeoutMS: 100) is exceeded.
151+
const { result } = await measureDuration(() => {
152+
return client.withSession(async s => {
153+
await s.withTransaction(async session => {
154+
await collection.insertOne({}, { session });
155+
});
156+
});
157+
});
158+
159+
// 3. Assert that the error is a timeout error wrapping the TransientTransactionError.
160+
expect(result).to.be.instanceOf(MongoOperationTimeoutError);
161+
expect((result as MongoOperationTimeoutError).cause).to.be.an('error');
162+
}
163+
);
164+
165+
// Case 2: If committing raises an error with the UnknownTransactionCommitResult label, and the
166+
// retry timeout has been exceeded, withTransaction should propagate the error (see Note 1) to
167+
// its caller.
168+
test(
169+
'commit UnknownTransactionCommitResult propagated as timeout error when retry timeout exceeded',
170+
{
171+
requires: {
172+
mongodb: '>=4.4',
173+
topology: '!single'
174+
}
175+
},
176+
async function () {
177+
// 1. Configure a failpoint that always fails commitTransaction with
178+
// UnknownTransactionCommitResult and blocks for 25ms to consume timeout budget.
179+
await configureFailPoint(this.configuration, {
180+
configureFailPoint: 'failCommand',
181+
mode: 'alwaysOn',
182+
data: {
183+
failCommands: ['commitTransaction'],
184+
blockConnection: true,
185+
blockTimeMS: 25,
186+
errorCode: 64,
187+
errorLabels: ['UnknownTransactionCommitResult']
188+
}
189+
});
190+
191+
// 2. Run withTransaction with a callback that performs an insert (succeeds).
192+
// The commit will always fail with UnknownTransactionCommitResult, triggering commit
193+
// retries until the timeout (timeoutMS: 100) is exceeded.
194+
const { result } = await measureDuration(() => {
195+
return client.withSession(async s => {
196+
await s.withTransaction(async session => {
197+
await collection.insertOne({}, { session });
198+
});
199+
});
200+
});
201+
202+
// 3. Assert that the error is a timeout error.
203+
expect(result).to.be.instanceOf(MongoOperationTimeoutError);
204+
}
205+
);
206+
207+
// Case 3: If committing raises an error with the TransientTransactionError label and the retry
208+
// timeout has been exceeded, withTransaction should propagate the error (see Note 1) to its
209+
// caller. This case may occur if the commit was internally retried against a new primary after a
210+
// failover and the second primary returned a NoSuchTransaction error response.
211+
test(
212+
'commit TransientTransactionError propagated as timeout error when retry timeout exceeded',
213+
{
214+
requires: {
215+
mongodb: '>=4.4',
216+
topology: '!single'
217+
}
218+
},
219+
async function () {
220+
// 1. Configure a failpoint that always fails commitTransaction with
221+
// TransientTransactionError (errorCode 251 = NoSuchTransaction) and blocks for 25ms
222+
// to consume timeout budget.
223+
await configureFailPoint(this.configuration, {
224+
configureFailPoint: 'failCommand',
225+
mode: 'alwaysOn',
226+
data: {
227+
failCommands: ['commitTransaction'],
228+
blockConnection: true,
229+
blockTimeMS: 25,
230+
errorCode: 251,
231+
errorLabels: ['TransientTransactionError']
232+
}
233+
});
234+
235+
// 2. Run withTransaction with a callback that performs an insert (succeeds).
236+
// The commit will always fail with TransientTransactionError, triggering full
237+
// transaction retries until the timeout (timeoutMS: 100) is exceeded.
238+
const { result } = await measureDuration(() => {
239+
return client.withSession(async s => {
240+
await s.withTransaction(async session => {
241+
await collection.insertOne({}, { session });
242+
});
243+
});
244+
});
245+
246+
// 3. Assert that the error is a timeout error wrapping the TransientTransactionError.
247+
expect(result).to.be.instanceOf(MongoOperationTimeoutError);
248+
expect((result as MongoOperationTimeoutError).cause).to.be.an('error');
249+
}
250+
);
251+
});

0 commit comments

Comments
 (0)