Skip to content

Commit

Permalink
test(NODE-3688): add cmap ignore ping events
Browse files Browse the repository at this point in the history
  • Loading branch information
@durran
durran committed Apr 12, 2022
1 parent ed56e31 commit 224e9cd
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 263 deletions.
8 changes: 6 additions & 2 deletions src/cmap/connect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
MongoNetworkError,
MongoNetworkTimeoutError,
MongoRuntimeError,
MongoServerError
MongoServerError,
needsRetryableWriteLabel
} from '../error';
import { Callback, ClientMetadata, HostAddress, makeClientMetadata, ns } from '../utils';
import { AuthContext, AuthProvider } from './auth/auth_provider';
Expand Down Expand Up @@ -185,7 +186,10 @@ function performInitialHandshake(
}
provider.auth(authContext, err => {
if (err) {
if (err instanceof MongoError) {
if (
err instanceof MongoError &&
needsRetryableWriteLabel(err, response.maxWireVersion)
) {
err.addErrorLabel(MongoErrorLabel.RetryableWriteError);
}
return callback(err);
Expand Down
15 changes: 6 additions & 9 deletions src/error.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -744,21 +744,18 @@ const RETRYABLE_WRITE_ERROR_CODES = new Set<number>([
]);

export function needsRetryableWriteLabel(error: Error, maxWireVersion: number): boolean {
if (maxWireVersion >= 9) {
// 4.4+ servers attach their own retryable write error
return false;
}
// pre-4.4 server, then the driver adds an error label for every valid case
// execute operation will only inspect the label, code/message logic is handled here

if (error instanceof MongoNetworkError) {
return true;
}

if (error instanceof MongoError && error.hasErrorLabel(MongoErrorLabel.RetryableWriteError)) {
// Before 4.4 the error label can be one way of identifying retry
// so we can return true if we have the label, but fall back to code checking below
return true;
if (
maxWireVersion >= 9 ||
(error instanceof MongoError && error.hasErrorLabel(MongoErrorLabel.RetryableWriteError))
) {
// If we already have the error label no need to add it again. 4.4+ servers add the label.
return false;
}

if (error instanceof MongoWriteConcernError) {
Expand Down
84 changes: 1 addition & 83 deletions test/integration/retryable-reads/retryable_reads.spec.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const { expect } = require('chai');
const path = require('path');
const { TestRunnerContext, generateTopologyTests } = require('../../tools/spec-runner');
const { loadSpecTests } = require('../../spec');
Expand Down Expand Up @@ -30,87 +29,6 @@ describe('Retryable Reads (legacy)', function () {
});
});

// These tests are skipped because the driver 1) executes a ping when connecting to
// an authenticated server and 2) command monitoring is at the connection level so
// when the handshake fails no command started event is emitted.
const SKIP = [
'find succeeds after retryable handshake network error',
'find succeeds after retryable handshake network error (ShutdownInProgress)'
];

describe('Retryable Reads (unified)', function () {
runUnifiedSuite(loadSpecTests(path.join('retryable-reads', 'unified')), SKIP);
});

describe('Retryable Reads Spec Manual Tests', function () {
context('retryable reads handshake failures', function () {
const metadata = {
requires: {
mongodb: '>=4.2.0',
auth: 'enabled',
topology: '!single'
}
};

const dbName = 'retryable-handshake-tests';
const collName = 'coll';
const inputDocs = [
{ _id: 1, x: 11 },
{ _id: 2, x: 22 },
{ _id: 3, x: 33 }
];
let client;
let db;
let coll;

beforeEach(async function () {
client = this.configuration.newClient({});
db = client.db(dbName);
coll = db.collection(collName);
await client.connect();
await coll.insertMany(inputDocs);
});

afterEach(async function () {
await db.admin().command({
configureFailPoint: 'failCommand',
mode: 'off'
});
await coll.drop();
await client.close();
});

context('when the handshake fails with a network error', function () {
// Manual implementation for: 'find succeeds after retryable handshake network error'
it('retries the read', metadata, async function () {
await db.admin().command({
configureFailPoint: 'failCommand',
mode: { times: 2 },
data: {
failCommands: ['saslContinue', 'ping'],
closeConnection: true
}
});
const documents = await coll.find({ _id: 2 }).toArray();
expect(documents).to.deep.equal([inputDocs[1]]);
});
});

context('when the handshake fails with shutdown in progress', function () {
// Manual implementation for:
// 'find succeeds after retryable handshake network error (ShutdownInProgress)'
it('retries the read', metadata, async function () {
await db.admin().command({
configureFailPoint: 'failCommand',
mode: { times: 2 },
data: {
failCommands: ['saslContinue', 'ping'],
errorCode: 91 // ShutdownInProgress
}
});
const documents = await coll.find({ _id: 2 }).toArray();
expect(documents).to.deep.equal([inputDocs[1]]);
});
});
});
runUnifiedSuite(loadSpecTests(path.join('retryable-reads', 'unified')));
});
79 changes: 1 addition & 78 deletions test/integration/retryable-writes/retryable_writes.spec.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,83 +199,6 @@ async function turnOffFailPoint(client, name) {
});
}

// These tests are skipped because the driver 1) executes a ping when connecting to
// an authenticated server and 2) command monitoring is at the connection level so
// when the handshake fails no command started event is emitted.
const SKIP = [
'InsertOne succeeds after retryable handshake error',
'InsertOne succeeds after retryable handshake error ShutdownInProgress'
];

describe('Retryable Writes (unified)', function () {
runUnifiedSuite(loadSpecTests(path.join('retryable-writes', 'unified')), SKIP);
});

describe('Retryable Writes Spec Manual Tests', function () {
context('retryable writes handshake failures', function () {
const metadata = {
requires: {
mongodb: '>=4.2.0',
auth: 'enabled',
topology: '!single'
}
};

const dbName = 'retryable-handshake-tests';
const collName = 'coll';
const docs = [{ _id: 1, x: 11 }];
let client;
let db;
let coll;

beforeEach(async function () {
client = this.configuration.newClient({});
db = client.db(dbName);
coll = db.collection(collName);
await client.connect();
await coll.insertMany(docs);
});

afterEach(async function () {
await db.admin().command({
configureFailPoint: 'failCommand',
mode: 'off'
});
await coll.drop();
await client.close();
});

context('when the handshake fails with a network error', function () {
// Manual implementation for: 'InsertOne succeeds after retryable handshake error'
(it as any)('retries the write', metadata, async function () {
await db.admin().command({
configureFailPoint: 'failCommand',
mode: { times: 2 },
data: {
failCommands: ['saslContinue', 'ping'],
closeConnection: true
}
});
const result = await coll.insertOne({ _id: 2, x: 22 });
expect(result.insertedId).to.equal(2);
});
});

context('when the handshake fails with shutdown in progress', function () {
// Manual implementation for:
// 'InsertOne succeeds after retryable handshake error ShutdownInProgress'
(it as any)('retries the write', metadata, async function () {
await db.admin().command({
configureFailPoint: 'failCommand',
mode: { times: 2 },
data: {
failCommands: ['saslContinue', 'ping'],
errorCode: 91 // ShutdownInProgress
}
});
const result = await coll.insertOne({ _id: 2, x: 22 });
expect(result.insertedId).to.equal(2);
});
});
});
runUnifiedSuite(loadSpecTests(path.join('retryable-writes', 'unified')));
});
89 changes: 1 addition & 88 deletions test/integration/transactions/transactions.spec.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,8 @@ class TransactionsRunnerContext extends TestRunnerContext {
}
}

// These tests are skipped because the driver 1) executes a ping when connecting to
// an authenticated server and 2) command monitoring is at the connection level so
// when the handshake fails no command started event is emitted.
// NOTE: these tests are skipped in the spec itself due to DRIVERS-2032 (unrelated to the above)
const SKIP = [
'AbortTransaction succeeds after handshake network error',
'CommitTransaction succeeds after handshake network error'
];

describe('Transactions Spec Unified Tests', function () {
runUnifiedSuite(loadSpecTests(path.join('transactions', 'unified')), SKIP);
runUnifiedSuite(loadSpecTests(path.join('transactions', 'unified')));
});

const SKIP_TESTS = [
Expand Down Expand Up @@ -128,81 +119,3 @@ describe('Transactions Spec Legacy Tests', function () {

generateTopologyTests(testSuites, testContext, testFilter);
});

describe('Transactions Spec Manual Tests', function () {
context('when the handshake fails with a network error', function () {
const metadata = {
requires: {
mongodb: '>=4.2.0',
auth: 'enabled',
topology: '!single'
}
};

const dbName = 'retryable-handshake-tests';
const collName = 'coll';
const docs = [{ _id: 1, x: 11 }];
let client;
let db;
let coll;
let session;

beforeEach(async function () {
if (process.env.SERVERLESS) {
this.currentTest.skipReason = 'Transaction tests cannot run against serverless';
this.skip();
}
client = this.configuration.newClient({});
db = client.db(dbName);
coll = db.collection(collName);
await client.connect();
await coll.insertMany(docs);
session = client.startSession();
session.startTransaction();
await coll.insertOne({ _id: 2, x: 22 }, { session });
});

afterEach(async function () {
await session.endSession();

await db.admin().command({
configureFailPoint: 'failCommand',
mode: 'off'
});
await coll.drop();
await client.close();
});

// Manual implementation for: 'AbortTransaction succeeds after handshake network error'
// NOTE: tests are skipped in the spec itself due to DRIVERS-2032 (unrelated to our reasons)
it('retries the abort', metadata, async function () {
await db.admin().command({
configureFailPoint: 'failCommand',
mode: { times: 2 },
data: {
failCommands: ['saslContinue', 'ping'],
closeConnection: true
}
});
await session.abortTransaction();
const doc = await coll.findOne({ _id: 2 });
expect(doc).to.not.exist;
});

// Manual implementation for: 'CommitTransaction succeeds after handshake network error'
// NOTE: tests are skipped in the spec itself due to DRIVERS-2032 (unrelated to our reasons)
it('retries the commit', metadata, async function () {
await db.admin().command({
configureFailPoint: 'failCommand',
mode: { times: 2 },
data: {
failCommands: ['saslContinue', 'ping'],
closeConnection: true
}
});
await session.commitTransaction();
const doc = await coll.findOne({ _id: 2 });
expect(doc.x).to.equal(22);
});
});
});
5 changes: 4 additions & 1 deletion test/spec/retryable-writes/unified/handshakeError.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,10 @@
"saslContinue",
"ping"
],
"errorCode": 91
"errorCode": 91,
"errorLabels": [
"RetryableWriteError"
]
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/spec/retryable-writes/unified/handshakeError.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ tests:
data:
failCommands: [saslContinue, ping]
errorCode: 91 # ShutdownInProgress

errorLabels: ["RetryableWriteError"]
- name: runCommand
object: *database0
arguments:
Expand Down
11 changes: 11 additions & 0 deletions test/tools/unified-spec-runner/runner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,14 @@ export async function runUnifiedTest(
}
}

let testPing = false;
for (const operation of test.operations) {
trace(operation.name);
// TODO: NODE-2149: Making connect optional should get rid of the initial ping in
// the driver so this block can then be removed.
if (operation.name === 'runCommand' && operation.arguments.commandName === 'ping') {
testPing = true;
}
try {
await executeOperationAndCheck(operation, entities, utilClient);
} catch (e) {
Expand Down Expand Up @@ -192,6 +198,11 @@ export async function runUnifiedTest(
const actualEvents =
eventType === 'cmap' ? clientCmapEvents.get(clientId) : clientCommandEvents.get(clientId);

// TODO: NODE-2149: Making connect optional should get rid of the initial ping in
// the driver so this block can then be removed.
if (eventType === 'cmap' && testPing) {
expectedEventList.events.push({ connectionCheckOutStartedEvent: {} });
}
expect(actualEvents, `No client entity found with id ${clientId}`).to.exist;
matchesEvents(expectedEventList.events, actualEvents, entities);
}
Expand Down
2 changes: 1 addition & 1 deletion test/unit/error.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ describe('MongoErrors', () => {
},
{
description: 'a MongoWriteConcernError with a RetryableWriteError label below server 4.4',
result: true,
result: false,
error: new MongoWriteConcernError({}, { errorLabels: ['RetryableWriteError'] }),
maxWireVersion: BELOW_4_4
},
Expand Down

0 comments on commit 224e9cd

Please sign in to comment.