Skip to content

Commit 2c1cb82

Browse files
committed
review comments
1 parent 1e58f14 commit 2c1cb82

File tree

3 files changed

+23
-23
lines changed

3 files changed

+23
-23
lines changed

src/cmap/wire_protocol/on_data.ts

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { type EventEmitter } from 'events';
22

3-
import { MongoOperationTimeoutError } from '../../error';
4-
import { type TimeoutContext, TimeoutError } from '../../timeout';
3+
import { type TimeoutContext } from '../../timeout';
54
import { List, promiseWithResolvers } from '../../utils';
65

76
/**
@@ -24,12 +23,6 @@ export function onData(
2423
emitter: EventEmitter,
2524
{ timeoutContext }: { timeoutContext?: TimeoutContext }
2625
) {
27-
// NOTE: Uncomment the block below to capture stack traces for debugging purposes
28-
// const capture: { stack?: string; name?: string; message?: string } = Object.create(null);
29-
// capture.name = 'MongoOperationTimeoutError';
30-
// capture.message = 'Timed out during socket read';
31-
// Error.captureStackTrace(capture);
32-
3326
// Setup pending events and pending promise lists
3427
/**
3528
* When the caller has not yet called .next(), we store the
@@ -112,15 +105,9 @@ export function onData(
112105

113106
function errorHandler(err: Error) {
114107
const promise = unconsumedPromises.shift();
115-
const timeoutError = TimeoutError.is(err)
116-
? new MongoOperationTimeoutError('Timed out during socket read')
117-
: undefined;
118-
119-
// NOTE: Uncomment this for debugging purposes
120-
// if (timeoutError instanceof MongoOperationTimeoutError && capture) timeoutError.stack = capture.stack;
121108

122-
if (promise != null) promise.reject(timeoutError ?? err);
123-
else error = timeoutError ?? err;
109+
if (promise != null) promise.reject(err);
110+
else error = err;
124111
void closeHandler();
125112
}
126113

src/cursor/abstract_cursor.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,11 +201,11 @@ export abstract class AbstractCursor<
201201
options.readPreference && options.readPreference instanceof ReadPreference
202202
? options.readPreference
203203
: ReadPreference.primary,
204-
...pluckBSONSerializeOptions(options)
204+
...pluckBSONSerializeOptions(options),
205+
timeoutMS: options.timeoutMS,
206+
tailable: options.tailable,
207+
awaitData: options.awaitData
205208
};
206-
this.cursorOptions.timeoutMS = options.timeoutMS;
207-
this.cursorOptions.tailable = options.tailable;
208-
this.cursorOptions.awaitData = options.awaitData;
209209
if (this.cursorOptions.timeoutMS != null) {
210210
if (options.timeoutMode == null) {
211211
if (options.tailable) {
@@ -784,8 +784,7 @@ export abstract class AbstractCursor<
784784
const getMoreOptions = {
785785
...this.cursorOptions,
786786
session: this.cursorSession,
787-
batchSize,
788-
omitMaxTimeMS: this.cursorOptions.omitMaxTimeMS
787+
batchSize
789788
};
790789

791790
if (
@@ -822,6 +821,8 @@ export abstract class AbstractCursor<
822821
}
823822
try {
824823
const state = await this._initialize(this.cursorSession);
824+
// Set omitMaxTimeMS to the value needed for subsequent getMore calls
825+
this.cursorOptions.omitMaxTimeMS = this.cursorOptions.timeoutMS != null;
825826
const response = state.response;
826827
this.selectedServer = state.server;
827828
this.cursorId = response.id;
@@ -865,7 +866,6 @@ export abstract class AbstractCursor<
865866

866867
// otherwise need to call getMore
867868
const batchSize = this.cursorOptions.batchSize || 1000;
868-
this.cursorOptions.omitMaxTimeMS = this.cursorOptions.timeoutMS != null;
869869
try {
870870
const response = await this.getMore(batchSize);
871871
this.cursorId = response.id;

test/integration/client-side-operations-timeout/node_csot.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ import { type FailPoint } from '../../tools/utils';
2626
const metadata = { requires: { mongodb: '>=4.4' } };
2727

2828
describe('CSOT driver tests', metadata, () => {
29+
// NOTE: minPoolSize here is set to ensure that connections are available when testing timeout
30+
// behaviour. This reduces flakiness in our tests since operations will not spend time
31+
// establishing connections, more closely mirroring long-running application behaviour
2932
const minPoolSize = 20;
3033

3134
describe('timeoutMS inheritance', () => {
@@ -651,6 +654,11 @@ describe('CSOT driver tests', metadata, () => {
651654
e => e
652655
);
653656
expect(maybeError).to.be.instanceOf(MongoOperationTimeoutError);
657+
658+
const finds = commandStarted.filter(x => x.commandName === 'find');
659+
const getMores = commandStarted.filter(x => x.commandName === 'getMore');
660+
expect(finds).to.have.lengthOf(1);
661+
expect(getMores).to.have.lengthOf(0);
654662
});
655663

656664
it('refreshes the timeout for subsequent getMores', async function () {
@@ -730,6 +738,11 @@ describe('CSOT driver tests', metadata, () => {
730738
e => e
731739
);
732740
expect(maybeError).to.be.instanceOf(MongoOperationTimeoutError);
741+
742+
const finds = commandStarted.filter(x => x.commandName === 'find');
743+
const getMores = commandStarted.filter(x => x.commandName === 'getMore');
744+
expect(finds).to.have.lengthOf(1);
745+
expect(getMores).to.have.lengthOf(0);
733746
});
734747

735748
it('refreshes the timeout for subsequent getMores', metadata, async function () {

0 commit comments

Comments
 (0)