Skip to content
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

feat(NODE-6312): add error transformation for server timeouts #4192

Merged
merged 6 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
25 changes: 25 additions & 0 deletions src/cmap/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from '../constants';
import {
MongoCompatibilityError,
MONGODB_ERROR_CODES,
MongoMissingDependencyError,
MongoNetworkError,
MongoNetworkTimeoutError,
Expand Down Expand Up @@ -537,6 +538,11 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
}

if (document.ok === 0) {
if (options.timeoutContext?.csotEnabled() && document.isMaxTimeExpiredError) {
throw new MongoOperationTimeoutError('Server reported a timeout error', {
cause: new MongoServerError((object ??= document.toObject(bsonOptions)))
aditi-khare-mongoDB marked this conversation as resolved.
Show resolved Hide resolved
});
}
throw new MongoServerError((object ??= document.toObject(bsonOptions)));
}

Expand Down Expand Up @@ -606,6 +612,25 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
): Promise<Document> {
this.throwIfAborted();
for await (const document of this.sendCommand(ns, command, options, responseType)) {
if (options.timeoutContext?.csotEnabled()) {
if (MongoDBResponse.is(document)) {
if (document.isMaxTimeExpiredError) {
throw new MongoOperationTimeoutError('Server reported a timeout error', {
cause: new MongoServerError(document.toObject())
});
}
} else {
aditi-khare-mongoDB marked this conversation as resolved.
Show resolved Hide resolved
if (
document?.writeErrors?.[0]?.code === MONGODB_ERROR_CODES.MaxTimeMSExpired ||
document?.writeConcernError?.code === MONGODB_ERROR_CODES.MaxTimeMSExpired
) {
throw new MongoOperationTimeoutError('Server reported a timeout error', {
cause: new MongoServerError(document)
});
}
}
}

return document;
}
throw new MongoUnexpectedServerResponseError('Unable to get response from server');
Expand Down
23 changes: 22 additions & 1 deletion src/cmap/wire_protocol/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
pluckBSONSerializeOptions,
type Timestamp
} from '../../bson';
import { MongoUnexpectedServerResponseError } from '../../error';
import { MONGODB_ERROR_CODES, MongoUnexpectedServerResponseError } from '../../error';
import { type ClusterTime } from '../../sdam/common';
import { decorateDecryptionResult, ns } from '../../utils';
import { type JSTypeOf, OnDemandDocument } from './on_demand/document';
Expand Down Expand Up @@ -104,6 +104,27 @@ export class MongoDBResponse extends OnDemandDocument {
// {ok:1}
static empty = new MongoDBResponse(new Uint8Array([13, 0, 0, 0, 16, 111, 107, 0, 1, 0, 0, 0, 0]));

/**
* Returns true iff:
* - ok is 0 and the top-level code === 50
* - ok is 1 and the writeErrors array contains a code === 50
* - ok is 1 and the writeConcern object contains a code === 50
*/
get isMaxTimeExpiredError() {
return (
// {ok: 0, code: 50 ... }
(this.ok === 0 && this.code === MONGODB_ERROR_CODES.MaxTimeMSExpired) ||
// {ok: 1, writeErrors: [{code: 50 ... }]}
(this.ok === 1 &&
this.get('writeErrors', BSONType.array)?.get(0, BSONType.object)?.getNumber('code') ===
MONGODB_ERROR_CODES.MaxTimeMSExpired) ||
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
// {ok: 1, writeConcernError: {code: 50 ... }}
(this.ok === 1 &&
this.get('writeConcernError', BSONType.object)?.getNumber('code') ===
MONGODB_ERROR_CODES.MaxTimeMSExpired)
);
}

/**
* Drivers can safely assume that the `recoveryToken` field is always a BSON document but drivers MUST NOT modify the
* contents of the document.
Expand Down
151 changes: 150 additions & 1 deletion test/integration/client-side-operations-timeout/node_csot.test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
/* Anything javascript specific relating to timeouts */
import { expect } from 'chai';
import * as semver from 'semver';
import * as sinon from 'sinon';

import {
BSON,
type ClientSession,
type Collection,
Connection,
type Db,
type FindCursor,
LEGACY_HELLO_COMMAND,
type MongoClient,
MongoOperationTimeoutError
MongoOperationTimeoutError,
MongoServerError
} from '../../mongodb';
import { type FailPoint } from '../../tools/utils';

describe('CSOT driver tests', () => {
describe('timeoutMS inheritance', () => {
Expand Down Expand Up @@ -161,4 +167,147 @@ describe('CSOT driver tests', () => {
});
});
});

describe(
'server-side maxTimeMS errors are transformed',
{ requires: { mongodb: '>=4.4' } },
() => {
let client: MongoClient;
let commandsSucceeded;
let commandsFailed;

beforeEach(async function () {
client = this.configuration.newClient({ timeoutMS: 500_000, monitorCommands: true });
commandsSucceeded = [];
commandsFailed = [];
client.on('commandSucceeded', event => {
if (event.commandName === 'configureFailPoint') return;
commandsSucceeded.push(event);
});
client.on('commandFailed', event => commandsFailed.push(event));
});

afterEach(async function () {
await client
.db()
.collection('a')
.drop()
.catch(() => null);
await client.close();
commandsSucceeded = undefined;
commandsFailed = undefined;
});

describe('when a maxTimeExpired error is returned at the top-level', () => {
// {ok: 0, code: 50, codeName: "MaxTimeMSExpired", errmsg: "operation time limit exceeded"}
const failpoint: FailPoint = {
configureFailPoint: 'failCommand',
mode: { times: 1 },
data: {
failCommands: ['ping'],
errorCode: 50
}
};

beforeEach(async () => {
await client.db('admin').command(failpoint);
});

afterEach(async () => {
await client.db('admin').command({ ...failpoint, mode: 'off' });
});

it('throws a MongoOperationTimeoutError error and emits command failed', async () => {
const error = await client
.db()
.command({ ping: 1 })
.catch(error => error);
expect(error).to.be.instanceOf(MongoOperationTimeoutError);
expect(error.cause).to.be.instanceOf(MongoServerError);
expect(error.cause).to.have.property('code', 50);

expect(commandsFailed).to.have.lengthOf(1);
expect(commandsFailed).to.have.nested.property('[0].failure.cause.code', 50);
});
});

describe('when a maxTimeExpired error is returned inside a writeErrors array', () => {
// Okay so allegedly this can never happen.
// But the spec says it can, so let's be defensive and support it.
// {ok: 1, writeErrors: [{code: 50, codeName: "MaxTimeMSExpired", errmsg: "operation time limit exceeded"}]}

beforeEach(async () => {
const writeErrorsReply = BSON.serialize({
ok: 1,
writeErrors: [
{ code: 50, codeName: 'MaxTimeMSExpired', errmsg: 'operation time limit exceeded' }
]
});
const commandSpy = sinon.spy(Connection.prototype, 'command');
const readManyStub = sinon
// @ts-expect-error: readMany is private
.stub(Connection.prototype, 'readMany')
.callsFake(async function* (...args) {
const realIterator = readManyStub.wrappedMethod.call(this, ...args);
const cmd = commandSpy.lastCall.args.at(1);
if ('giveMeWriteErrors' in cmd) {
await realIterator.next().catch(() => null); // dismiss response
yield { parse: () => writeErrorsReply };
} else {
yield (await realIterator.next()).value;
}
});
});

afterEach(() => sinon.restore());

it('throws a MongoOperationTimeoutError error and emits command succeeded', async () => {
const error = await client
.db('admin')
.command({ giveMeWriteErrors: 1 })
.catch(error => error);
expect(error).to.be.instanceOf(MongoOperationTimeoutError);
expect(error.cause).to.be.instanceOf(MongoServerError);
expect(error.cause).to.have.nested.property('writeErrors[0].code', 50);

expect(commandsSucceeded).to.have.lengthOf(1);
expect(commandsSucceeded).to.have.nested.property('[0].reply.writeErrors[0].code', 50);
});
});

describe('when a maxTimeExpired error is returned inside a writeConcernError embedded document', () => {
// {ok: 1, writeConcernError: {code: 50, codeName: "MaxTimeMSExpired"}}
const failpoint: FailPoint = {
configureFailPoint: 'failCommand',
mode: { times: 1 },
data: {
failCommands: ['insert'],
writeConcernError: { code: 50, errmsg: 'times up buster', errorLabels: [] }
}
};

beforeEach(async () => {
await client.db('admin').command(failpoint);
});

afterEach(async () => {
await client.db('admin').command({ ...failpoint, mode: 'off' });
});

it('throws a MongoOperationTimeoutError error and emits command succeeded', async () => {
const error = await client
.db()
.collection('a')
.insertOne({})
.catch(error => error);
expect(error).to.be.instanceOf(MongoOperationTimeoutError);
expect(error.cause).to.be.instanceOf(MongoServerError);
expect(error.cause).to.have.nested.property('writeConcernError.code', 50);

expect(commandsSucceeded).to.have.lengthOf(1);
expect(commandsSucceeded).to.have.nested.property('[0].reply.writeConcernError.code', 50);
});
});
}
);
});