Skip to content

Commit 85c086d

Browse files
committed
chore: adjust timeouts and correct promise chain
1 parent 6d9266e commit 85c086d

File tree

5 files changed

+71
-45
lines changed

5 files changed

+71
-45
lines changed

src/cmap/wire_protocol/on_data.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ export function onData(
105105
function errorHandler(err: Error) {
106106
const promise = unconsumedPromises.shift();
107107
const timeoutError = TimeoutError.is(err)
108-
? new MongoOperationTimeoutError('Timed out during socket read')
108+
? new MongoOperationTimeoutError(`Timed out during socket read (${err.duration}ms)`)
109109
: undefined;
110110

111111
if (promise != null) promise.reject(timeoutError ?? err);

src/sessions.ts

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -281,9 +281,9 @@ export class ClientSession
281281
try {
282282
if (this.inTransaction()) {
283283
if (typeof options?.timeoutMS === 'number') {
284-
await this.abortTransaction({ timeoutMS: options.timeoutMS });
284+
await endTransaction(this, 'abortTransaction', { timeoutMS: options.timeoutMS });
285285
} else {
286-
await this.abortTransaction();
286+
await endTransaction(this, 'abortTransaction');
287287
}
288288
}
289289
if (!this.hasEnded) {
@@ -470,7 +470,11 @@ export class ClientSession
470470
* @param options - Optional options, can be used to override `defaultTimeoutMS`.
471471
*/
472472
async abortTransaction(options?: { timeoutMS: number }): Promise<void> {
473-
return await endTransaction(this, 'abortTransaction', options);
473+
try {
474+
return await endTransaction(this, 'abortTransaction', options);
475+
} catch (error) {
476+
squashError(error);
477+
}
474478
}
475479

476480
/**
@@ -664,7 +668,7 @@ async function attemptTransaction<T>(
664668

665669
if (!isPromiseLike(promise)) {
666670
try {
667-
await session.abortTransaction();
671+
await endTransaction(session, 'abortTransaction');
668672
} catch (error) {
669673
squashError(error);
670674
}
@@ -681,7 +685,7 @@ async function attemptTransaction<T>(
681685
return await attemptTransactionCommit(session, startTime, fn, result, options);
682686
} catch (err) {
683687
if (session.inTransaction()) {
684-
await session.abortTransaction();
688+
await endTransaction(session, 'abortTransaction');
685689
}
686690

687691
if (
@@ -768,20 +772,20 @@ async function endTransaction(
768772
}
769773

770774
if (commandName === 'commitTransaction' && session.transaction.options.maxTimeMS) {
771-
Object.assign(command, { maxTimeMS: session.transaction.options.maxTimeMS });
775+
command.maxTimeMS = session.transaction.options.maxTimeMS;
772776
}
773777

774778
if (session.transaction.recoveryToken) {
775779
command.recoveryToken = session.transaction.recoveryToken;
776780
}
777781

778782
const timeoutMS =
779-
'timeoutMS' in options && typeof options.timeoutMS === 'number'
783+
'timeoutMS' in options && typeof options.timeoutMS === 'number' // timeoutMS provided to method
780784
? options.timeoutMS
781-
: typeof session.timeoutMS === 'number'
785+
: session.timeoutContext?.csotEnabled() // timeoutMS provided to withTxn, ctx created
786+
? session.timeoutContext.timeoutMS // refresh!
787+
: typeof session.timeoutMS === 'number' // timeoutMS inherited from MamaClient
782788
? session.timeoutMS
783-
: session.timeoutContext?.csotEnabled()
784-
? session.timeoutContext.timeoutMS
785789
: null;
786790

787791
const timeoutContext =
@@ -795,20 +799,19 @@ async function endTransaction(
795799
});
796800

797801
try {
798-
// send the command
799-
await executeOperation(
800-
session.client,
801-
new RunAdminCommandOperation(command, {
802-
session,
803-
readPreference: ReadPreference.primary,
804-
bypassPinningCheck: true
805-
}),
806-
timeoutContext
807-
);
802+
const adminCommand = new RunAdminCommandOperation(command, {
803+
session,
804+
readPreference: ReadPreference.primary,
805+
bypassPinningCheck: true
806+
});
807+
808+
await executeOperation(session.client, adminCommand, timeoutContext);
809+
808810
if (command.abortTransaction) {
809811
// always unpin on abort regardless of command outcome
810812
session.unpin();
811813
}
814+
812815
if (commandName !== 'commitTransaction') {
813816
session.transaction.transition(TxnState.TRANSACTION_ABORTED);
814817
if (session.loadBalanced) {

src/timeout.ts

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@ import { csotMin, noop } from './utils';
55

66
/** @internal */
77
export class TimeoutError extends Error {
8+
duration: number;
89
override get name(): 'TimeoutError' {
910
return 'TimeoutError';
1011
}
1112

12-
constructor(message: string, options?: { cause?: Error }) {
13+
constructor(message: string, options: { cause?: Error; duration: number }) {
1314
super(message, options);
15+
this.duration = options.duration;
1416
}
1517

1618
static is(error: unknown): error is TimeoutError {
@@ -52,7 +54,12 @@ export class Timeout extends Promise<never> {
5254
}
5355

5456
/** Create a new timeout that expires in `duration` ms */
55-
private constructor(executor: Executor = () => null, duration: number, unref = true) {
57+
private constructor(
58+
executor: Executor = () => null,
59+
duration: number,
60+
unref = true,
61+
rejection: Error | null = null
62+
) {
5663
let reject!: Reject;
5764
if (duration < 0) {
5865
throw new MongoInvalidArgumentError('Cannot create a Timeout with a negative duration');
@@ -71,13 +78,15 @@ export class Timeout extends Promise<never> {
7178
this.id = setTimeout(() => {
7279
this.ended = Math.trunc(performance.now());
7380
this.timedOut = true;
74-
reject(new TimeoutError(`Expired after ${duration}ms`));
81+
reject(new TimeoutError(`Expired after ${duration}ms`, { duration }));
7582
}, this.duration);
7683
if (typeof this.id.unref === 'function' && unref) {
7784
// Ensure we do not keep the Node.js event loop running
7885
this.id.unref();
7986
}
8087
}
88+
89+
if (rejection != null) reject(rejection);
8190
}
8291

8392
/**
@@ -90,7 +99,7 @@ export class Timeout extends Promise<never> {
9099
}
91100

92101
throwIfExpired(): void {
93-
if (this.timedOut) throw new TimeoutError('Timed out');
102+
if (this.timedOut) throw new TimeoutError('Timed out', { duration: this.duration });
94103
}
95104

96105
public static expires(durationMS: number, unref?: boolean): Timeout {
@@ -108,6 +117,10 @@ export class Timeout extends Promise<never> {
108117
typeof timeout.then === 'function'
109118
);
110119
}
120+
121+
static override reject(rejection?: Error): Timeout {
122+
return new Timeout(undefined, 0, true, rejection);
123+
}
111124
}
112125

113126
/** @internal */
@@ -218,8 +231,8 @@ export class CSOTTimeoutContext extends TimeoutContext {
218231
if (typeof this._serverSelectionTimeout !== 'object' || this._serverSelectionTimeout?.cleared) {
219232
const { remainingTimeMS, serverSelectionTimeoutMS } = this;
220233
if (remainingTimeMS <= 0)
221-
throw new MongoOperationTimeoutError(
222-
`Timed out in server selection after ${this.timeoutMS}ms`
234+
return Timeout.reject(
235+
new MongoOperationTimeoutError(`Timed out in server selection after ${this.timeoutMS}ms`)
223236
);
224237
const usingServerSelectionTimeoutMS =
225238
serverSelectionTimeoutMS !== 0 &&
@@ -247,8 +260,10 @@ export class CSOTTimeoutContext extends TimeoutContext {
247260
// null or Timeout
248261
this._connectionCheckoutTimeout = this._serverSelectionTimeout;
249262
} else {
250-
throw new MongoRuntimeError(
251-
'Unreachable. If you are seeing this error, please file a ticket on the NODE driver project on Jira'
263+
return Timeout.reject(
264+
new MongoRuntimeError(
265+
'Unreachable. If you are seeing this error, please file a ticket on the NODE driver project on Jira'
266+
)
252267
);
253268
}
254269
}
@@ -259,14 +274,14 @@ export class CSOTTimeoutContext extends TimeoutContext {
259274
const { remainingTimeMS } = this;
260275
if (!Number.isFinite(remainingTimeMS)) return null;
261276
if (remainingTimeMS > 0) return Timeout.expires(remainingTimeMS);
262-
throw new MongoOperationTimeoutError('Timed out before socket write');
277+
return Timeout.reject(new MongoOperationTimeoutError('Timed out before socket write'));
263278
}
264279

265280
get timeoutForSocketRead(): Timeout | null {
266281
const { remainingTimeMS } = this;
267282
if (!Number.isFinite(remainingTimeMS)) return null;
268283
if (remainingTimeMS > 0) return Timeout.expires(remainingTimeMS);
269-
throw new MongoOperationTimeoutError('Timed out before socket read');
284+
return Timeout.reject(new MongoOperationTimeoutError('Timed out before socket read'));
270285
}
271286
}
272287

src/utils.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -528,12 +528,6 @@ export function resolveOptions<T extends CommandOperationOptions>(
528528
result.readPreference = readPreference;
529529
}
530530

531-
if (session?.explicit && session.timeoutMS != null && options?.timeoutMS != null) {
532-
throw new MongoInvalidArgumentError(
533-
'Do not specify timeoutMS on operation if already specified on an explicit session'
534-
);
535-
}
536-
537531
const timeoutMS = options?.timeoutMS;
538532

539533
result.timeoutMS = timeoutMS ?? parent?.timeoutMS;

test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,11 @@ describe('CSOT spec prose tests', function () {
633633
const failpoint: FailPoint = {
634634
configureFailPoint: 'failCommand',
635635
mode: { times: 1 },
636-
data: { failCommands: ['abortTransaction'], blockConnection: true, blockTimeMS: 15 }
636+
data: {
637+
failCommands: ['abortTransaction'],
638+
blockConnection: true,
639+
blockTimeMS: 60
640+
}
637641
};
638642

639643
beforeEach(async function () {
@@ -653,13 +657,18 @@ describe('CSOT spec prose tests', function () {
653657

654658
let client: MongoClient;
655659

656-
afterEach(async () => {
660+
afterEach(async function () {
661+
if (semver.satisfies(this.configuration.version, '>=4.4')) {
662+
const internalClient = this.configuration.newClient();
663+
await internalClient.db('admin').command({ ...failpoint, mode: 'off' });
664+
await internalClient.close();
665+
}
657666
await client?.close();
658667
});
659668

660669
describe('when timeoutMS is provided to the client', () => {
661670
it('throws a timeout error from endSession', async function () {
662-
client = this.configuration.newClient({ timeoutMS: 10 });
671+
client = this.configuration.newClient({ timeoutMS: 50, monitorCommands: true });
663672
const coll = client.db('db').collection('coll');
664673
const session = client.startSession();
665674
session.startTransaction();
@@ -673,7 +682,7 @@ describe('CSOT spec prose tests', function () {
673682
it('throws a timeout error from endSession', async function () {
674683
client = this.configuration.newClient();
675684
const coll = client.db('db').collection('coll');
676-
const session = client.startSession({ defaultTimeoutMS: 10 });
685+
const session = client.startSession({ defaultTimeoutMS: 50 });
677686
session.startTransaction();
678687
await coll.insertOne({ x: 1 }, { session });
679688
const error = await session.endSession().catch(error => error);
@@ -688,7 +697,7 @@ describe('CSOT spec prose tests', function () {
688697
const session = client.startSession();
689698
session.startTransaction();
690699
await coll.insertOne({ x: 1 }, { session });
691-
const error = await session.endSession({ timeoutMS: 10 }).catch(error => error);
700+
const error = await session.endSession({ timeoutMS: 50 }).catch(error => error);
692701
expect(error).to.be.instanceOf(MongoOperationTimeoutError);
693702
});
694703
});
@@ -736,7 +745,7 @@ describe('CSOT spec prose tests', function () {
736745
data: {
737746
failCommands: ['insert', 'abortTransaction'],
738747
blockConnection: true,
739-
blockTimeMS: 15
748+
blockTimeMS: 60
740749
}
741750
};
742751

@@ -757,15 +766,20 @@ describe('CSOT spec prose tests', function () {
757766

758767
let client: MongoClient;
759768

760-
afterEach(async () => {
769+
afterEach(async function () {
770+
if (semver.satisfies(this.configuration.version, '>=4.4')) {
771+
const internalClient = this.configuration.newClient();
772+
await internalClient.db('admin').command({ ...failpoint, mode: 'off' });
773+
await internalClient.close();
774+
}
761775
await client?.close();
762776
});
763777

764778
it('timeoutMS is refreshed for abortTransaction', async function () {
765779
const commandsFailed = [];
766780

767781
client = this.configuration
768-
.newClient({ timeoutMS: 10, monitorCommands: true })
782+
.newClient({ timeoutMS: 50, monitorCommands: true })
769783
.on('commandFailed', e => commandsFailed.push(e));
770784

771785
const coll = client.db('db').collection('coll');

0 commit comments

Comments
 (0)