Skip to content

Commit 3138615

Browse files
committed
wip
1 parent 247284e commit 3138615

File tree

7 files changed

+100
-74
lines changed

7 files changed

+100
-74
lines changed

src/sdam/topology_description.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { EJSON, type ObjectId } from '../bson';
22
import * as WIRE_CONSTANTS from '../cmap/wire_protocol/constants';
3-
import { type MongoError, MongoRuntimeError } from '../error';
3+
import { MongoError, MongoRuntimeError } from '../error';
44
import { compareObjectId, shuffle } from '../utils';
55
import { ServerType, TopologyType } from './common';
66
import { ServerDescription } from './server_description';
@@ -400,7 +400,9 @@ function updateRsFromPrimary(
400400
// replace serverDescription with a default ServerDescription of type "Unknown"
401401
serverDescriptions.set(
402402
serverDescription.address,
403-
new ServerDescription(serverDescription.address)
403+
new ServerDescription(serverDescription.address, undefined, {
404+
error: new MongoError('stale')
405+
})
404406
);
405407

406408
return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
@@ -416,7 +418,9 @@ function updateRsFromPrimary(
416418
// this primary is stale, we must remove it
417419
serverDescriptions.set(
418420
serverDescription.address,
419-
new ServerDescription(serverDescription.address)
421+
new ServerDescription(serverDescription.address, undefined, {
422+
error: new MongoError('stale')
423+
})
420424
);
421425

422426
return [checkHasPrimary(serverDescriptions), setName, maxSetVersion, maxElectionId];
@@ -438,7 +442,10 @@ function updateRsFromPrimary(
438442
for (const [address, server] of serverDescriptions) {
439443
if (server.type === ServerType.RSPrimary && server.address !== serverDescription.address) {
440444
// Reset old primary's type to Unknown.
441-
serverDescriptions.set(address, new ServerDescription(server.address));
445+
serverDescriptions.set(
446+
address,
447+
new ServerDescription(server.address, undefined, { error: new MongoError('stale') })
448+
);
442449

443450
// There can only be one primary
444451
break;

test/spec/server-discovery-and-monitoring/rs/new_primary.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
"servers": {
5959
"a:27017": {
6060
"type": "Unknown",
61+
"error": "stale",
6162
"setName": null
6263
},
6364
"b:27017": {

test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
"servers": {
4848
"a:27017": {
4949
"type": "Unknown",
50+
"error": {"message": "stale"},
5051
"setName": null,
5152
"electionId": null
5253
},

test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
"servers": {
4848
"a:27017": {
4949
"type": "Unknown",
50+
"error": { "message": "stale" },
5051
"setName": null,
5152
"electionId": null
5253
},

test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ phases: [
3535
servers: {
3636
"a:27017": {
3737
type: "Unknown",
38+
error: "stale",
3839
setName: ,
3940
electionId:
4041
},

test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@
6464
"servers": {
6565
"a:27017": {
6666
"type": "Unknown",
67+
"error": {
68+
"message": "stale"
69+
},
6770
"setName": null,
6871
"electionId": null
6972
},

test/unit/assorted/server_discovery_and_monitoring.spec.test.ts

Lines changed: 82 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
isRecord,
1111
MongoClient,
1212
MongoCompatibilityError,
13+
MongoError,
1314
MongoNetworkError,
1415
MongoNetworkTimeoutError,
1516
MongoServerError,
@@ -25,6 +26,7 @@ import {
2526
ServerHeartbeatStartedEvent,
2627
ServerHeartbeatSucceededEvent,
2728
ServerOpeningEvent,
29+
squashError,
2830
Topology,
2931
TOPOLOGY_CLOSED,
3032
TOPOLOGY_DESCRIPTION_CHANGED,
@@ -108,6 +110,7 @@ interface MonitoringOutcome {
108110
interface OutcomeServerDescription {
109111
type?: string;
110112
setName?: string;
113+
error?: { message: string };
111114
setVersion?: number;
112115
electionId?: ObjectId | null;
113116
logicalSessionTimeoutMinutes?: number;
@@ -268,6 +271,7 @@ function normalizeServerDescription(serverDescription) {
268271
// it as `Unknown`.
269272
serverDescription.type = 'Unknown';
270273
}
274+
serverDescription.error == serverDescription?.error?.message;
271275

272276
return serverDescription;
273277
}
@@ -322,84 +326,88 @@ async function executeSDAMTest(testData: SDAMTest) {
322326
// connect the topology
323327
await client.connect();
324328

325-
for (const phase of testData.phases) {
326-
// Determine which of the two kinds of phases we're running
327-
if ('responses' in phase && phase.responses != null) {
328-
// phase with responses for hello simulations
329-
for (const [address, hello] of phase.responses) {
330-
client.topology.serverUpdateHandler(new ServerDescription(address, hello));
331-
}
332-
} else if ('applicationErrors' in phase && phase.applicationErrors) {
333-
// phase with applicationErrors simulating error's from network, timeouts, server
334-
for (const appError of phase.applicationErrors) {
335-
// Stub will return appError to SDAM machinery
336-
const checkOutStub = sinon
337-
.stub(ConnectionPool.prototype, 'checkOut')
338-
.callsFake(checkoutStubImpl(appError));
339-
340-
const server = client.topology.s.servers.get(appError.address);
341-
342-
// Run a dummy command to encounter the error
343-
const res = server.command.bind(server)(ns('admin.$cmd'), { ping: 1 }, {});
344-
const thrownError = await res.catch(error => error);
345-
346-
// Restore the stub before asserting anything in case of errors
347-
checkOutStub.restore();
348-
349-
const isApplicationError = error => {
350-
// These errors all come from the withConnection stub
351-
return (
352-
error instanceof MongoNetworkError ||
353-
error instanceof MongoNetworkTimeoutError ||
354-
error instanceof MongoServerError
355-
);
356-
};
357-
expect(
358-
thrownError,
359-
`expected the error thrown to be one of MongoNetworkError, MongoNetworkTimeoutError or MongoServerError (referred to in the spec as an "Application Error") got ${thrownError.name} ${thrownError.stack}`
360-
).to.satisfy(isApplicationError);
329+
try {
330+
for (const phase of testData.phases) {
331+
// Determine which of the two kinds of phases we're running
332+
if ('responses' in phase && phase.responses != null) {
333+
// phase with responses for hello simulations
334+
for (const [address, hello] of phase.responses) {
335+
client.topology.serverUpdateHandler(new ServerDescription(address, hello));
336+
}
337+
} else if ('applicationErrors' in phase && phase.applicationErrors) {
338+
// phase with applicationErrors simulating error's from network, timeouts, server
339+
for (const appError of phase.applicationErrors) {
340+
// Stub will return appError to SDAM machinery
341+
const checkOutStub = sinon
342+
.stub(ConnectionPool.prototype, 'checkOut')
343+
.callsFake(checkoutStubImpl(appError));
344+
345+
const server = client.topology.s.servers.get(appError.address);
346+
347+
// Run a dummy command to encounter the error
348+
const res = server.command.bind(server)(ns('admin.$cmd'), { ping: 1 }, {});
349+
const thrownError = await res.catch(error => error);
350+
351+
// Restore the stub before asserting anything in case of errors
352+
checkOutStub.restore();
353+
354+
const isApplicationError = error => {
355+
// These errors all come from the withConnection stub
356+
return (
357+
error instanceof MongoNetworkError ||
358+
error instanceof MongoNetworkTimeoutError ||
359+
error instanceof MongoServerError
360+
);
361+
};
362+
expect(
363+
thrownError,
364+
`expected the error thrown to be one of MongoNetworkError, MongoNetworkTimeoutError or MongoServerError (referred to in the spec as an "Application Error") got ${thrownError.name} ${thrownError.stack}`
365+
).to.satisfy(isApplicationError);
366+
}
367+
} else if (phase.outcome != null && Object.keys(phase).length === 1) {
368+
// Load Balancer SDAM tests have no "work" to be done for the phase
369+
} else {
370+
expect.fail(ejson`Unknown phase shape - ${phase}`);
361371
}
362-
} else if (phase.outcome != null && Object.keys(phase).length === 1) {
363-
// Load Balancer SDAM tests have no "work" to be done for the phase
364-
} else {
365-
expect.fail(ejson`Unknown phase shape - ${phase}`);
366-
}
367372

368-
if ('outcome' in phase && phase.outcome != null) {
369-
if (isMonitoringOutcome(phase.outcome)) {
370-
// Test for monitoring events
371-
const expectedEvents = convertOutcomeEvents(phase.outcome.events);
373+
if ('outcome' in phase && phase.outcome != null) {
374+
if (isMonitoringOutcome(phase.outcome)) {
375+
// Test for monitoring events
376+
const expectedEvents = convertOutcomeEvents(phase.outcome.events);
372377

373-
expect(events).to.have.length(expectedEvents.length);
374-
for (const [i, actualEvent] of Object.entries(events)) {
375-
const actualEventClone = cloneForCompare(actualEvent);
376-
expect(actualEventClone).to.matchMongoSpec(expectedEvents[i]);
377-
}
378-
} else if (isTopologyDescriptionOutcome(phase.outcome)) {
379-
// Test for SDAM machinery correctly changing the topology type among other properties
380-
assertTopologyDescriptionOutcomeExpectations(client.topology, phase.outcome);
381-
if (phase.outcome.compatible === false) {
382-
// driver specific error throwing
383-
if (testData.description === 'Multiple mongoses with large minWireVersion') {
384-
// TODO(DRIVERS-2250): There is test bug that causes two errors
385-
// this will start failing when the test is synced and fixed
386-
expect(errorsThrown).to.have.lengthOf(2);
378+
expect(events).to.have.length(expectedEvents.length);
379+
for (const [i, actualEvent] of Object.entries(events)) {
380+
const actualEventClone = cloneForCompare(actualEvent);
381+
expect(actualEventClone).to.matchMongoSpec(expectedEvents[i]);
382+
}
383+
} else if (isTopologyDescriptionOutcome(phase.outcome)) {
384+
// Test for SDAM machinery correctly changing the topology type among other properties
385+
assertTopologyDescriptionOutcomeExpectations(client.topology, phase.outcome);
386+
if (phase.outcome.compatible === false) {
387+
// driver specific error throwing
388+
if (testData.description === 'Multiple mongoses with large minWireVersion') {
389+
// TODO(DRIVERS-2250): There is test bug that causes two errors
390+
// this will start failing when the test is synced and fixed
391+
expect(errorsThrown).to.have.lengthOf(2);
392+
} else {
393+
expect(errorsThrown).to.have.lengthOf(1);
394+
}
395+
expect(errorsThrown[0]).to.be.instanceOf(MongoCompatibilityError);
396+
expect(errorsThrown[0].message).to.match(/but this version of the driver/);
387397
} else {
388-
expect(errorsThrown).to.have.lengthOf(1);
398+
// unset or true means no errors should be thrown
399+
expect(errorsThrown).to.be.empty;
389400
}
390-
expect(errorsThrown[0]).to.be.instanceOf(MongoCompatibilityError);
391-
expect(errorsThrown[0].message).to.match(/but this version of the driver/);
392401
} else {
393-
// unset or true means no errors should be thrown
394-
expect(errorsThrown).to.be.empty;
402+
expect.fail(ejson`Unknown outcome shape - ${phase.outcome}`);
395403
}
396-
} else {
397-
expect.fail(ejson`Unknown outcome shape - ${phase.outcome}`);
398-
}
399404

400-
events = [];
401-
errorsThrown = [];
405+
events = [];
406+
errorsThrown = [];
407+
}
402408
}
409+
} finally {
410+
await client.close().catch(squashError);
403411
}
404412
}
405413

@@ -464,8 +472,12 @@ function assertTopologyDescriptionOutcomeExpectations(
464472
if (WIRE_VERSION_KEYS.has(expectedKey) && expectedValue === null) {
465473
// For wireVersion keys we default to zero instead of null
466474
expect(actualServer).to.have.property(expectedKey, 0);
467-
} else {
475+
} else if (expectedKey !== 'error') {
468476
expect(actualServer).to.have.deep.property(expectedKey, expectedValue);
477+
} else {
478+
// Check that if we have
479+
expect(actualServer).to.have.deep.property(expectedKey).instanceof(MongoError);
480+
expect(actualServer).property(expectedKey).property('message').includes(expectedValue);
469481
}
470482
}
471483
}

0 commit comments

Comments
 (0)