Skip to content

refactor(NODE-3788): update names of offensive error codes #3063

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Dec 6, 2021
Merged

refactor(NODE-3788): update names of offensive error codes #3063

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3f1bfcc
Update references to master in error.ts
ljhaywar Dec 2, 2021
bcdbdaa
fix test that checks error message
ljhaywar Dec 2, 2021
6337e56
Use original error messages & abstract to constants
ljhaywar Dec 3, 2021
eeb135e
Add unit tests for isSDAMUnrecoverableError
ljhaywar Dec 3, 2021
bc47e37
Add comments
ljhaywar Dec 3, 2021
3875192
Code review: create constants
ljhaywar Dec 6, 2021
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
70 changes: 47 additions & 23 deletions src/error.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,27 @@ export type AnyError = MongoError | Error;
/** @internal */
const kErrorLabels = Symbol('errorLabels');

/**
* @internal
* The legacy error message from the server that indicates the node is not a writable primary
* https://github.com/mongodb/specifications/blob/b07c26dc40d04ac20349f989db531c9845fdd755/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst#not-writable-primary-and-node-is-recovering
*/
export const LEGACY_NOT_WRITABLE_PRIMARY_ERROR_MESSAGE = 'not master';

/**
* @internal
* The legacy error message from the server that indicates the node is not a primary or secondary
* https://github.com/mongodb/specifications/blob/b07c26dc40d04ac20349f989db531c9845fdd755/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst#not-writable-primary-and-node-is-recovering
*/
export const LEGACY_NOT_PRIMARY_OR_SECONDARY_ERROR_MESSAGE = 'not master or secondary';

/**
* @internal
* The error message from the server that indicates the node is recovering
* https://github.com/mongodb/specifications/blob/b07c26dc40d04ac20349f989db531c9845fdd755/source/server-discovery-and-monitoring/server-discovery-and-monitoring.rst#not-writable-primary-and-node-is-recovering
*/
export const NODE_IS_RECOVERING_ERROR_MESSAGE = 'node is recovering';

/** @internal MongoDB Error Codes */
export const MONGODB_ERROR_CODES = Object.freeze({
HostUnreachable: 6,
Expand All @@ -17,11 +38,11 @@ export const MONGODB_ERROR_CODES = Object.freeze({
PrimarySteppedDown: 189,
ExceededTimeLimit: 262,
SocketException: 9001,
NotMaster: 10107,
NotWritablePrimary: 10107,
InterruptedAtShutdown: 11600,
InterruptedDueToReplStateChange: 11602,
NotMasterNoSlaveOk: 13435,
NotMasterOrSecondary: 13436,
NotPrimaryNoSecondaryOk: 13435,
NotPrimaryOrSecondary: 13436,
StaleShardVersion: 63,
StaleEpoch: 150,
StaleConfig: 13388,
Expand All @@ -46,11 +67,11 @@ export const GET_MORE_RESUMABLE_CODES = new Set<number>([
MONGODB_ERROR_CODES.PrimarySteppedDown,
MONGODB_ERROR_CODES.ExceededTimeLimit,
MONGODB_ERROR_CODES.SocketException,
MONGODB_ERROR_CODES.NotMaster,
MONGODB_ERROR_CODES.NotWritablePrimary,
MONGODB_ERROR_CODES.InterruptedAtShutdown,
MONGODB_ERROR_CODES.InterruptedDueToReplStateChange,
MONGODB_ERROR_CODES.NotMasterNoSlaveOk,
MONGODB_ERROR_CODES.NotMasterOrSecondary,
MONGODB_ERROR_CODES.NotPrimaryNoSecondaryOk,
MONGODB_ERROR_CODES.NotPrimaryOrSecondary,
MONGODB_ERROR_CODES.StaleShardVersion,
MONGODB_ERROR_CODES.StaleEpoch,
MONGODB_ERROR_CODES.StaleConfig,
Expand Down Expand Up @@ -675,19 +696,19 @@ const RETRYABLE_ERROR_CODES = new Set<number>([
MONGODB_ERROR_CODES.ShutdownInProgress,
MONGODB_ERROR_CODES.PrimarySteppedDown,
MONGODB_ERROR_CODES.SocketException,
MONGODB_ERROR_CODES.NotMaster,
MONGODB_ERROR_CODES.NotWritablePrimary,
MONGODB_ERROR_CODES.InterruptedAtShutdown,
MONGODB_ERROR_CODES.InterruptedDueToReplStateChange,
MONGODB_ERROR_CODES.NotMasterNoSlaveOk,
MONGODB_ERROR_CODES.NotMasterOrSecondary
MONGODB_ERROR_CODES.NotPrimaryNoSecondaryOk,
MONGODB_ERROR_CODES.NotPrimaryOrSecondary
]);

const RETRYABLE_WRITE_ERROR_CODES = new Set<number>([
MONGODB_ERROR_CODES.InterruptedAtShutdown,
MONGODB_ERROR_CODES.InterruptedDueToReplStateChange,
MONGODB_ERROR_CODES.NotMaster,
MONGODB_ERROR_CODES.NotMasterNoSlaveOk,
MONGODB_ERROR_CODES.NotMasterOrSecondary,
MONGODB_ERROR_CODES.NotWritablePrimary,
MONGODB_ERROR_CODES.NotPrimaryNoSecondaryOk,
MONGODB_ERROR_CODES.NotPrimaryOrSecondary,
MONGODB_ERROR_CODES.PrimarySteppedDown,
MONGODB_ERROR_CODES.ShutdownInProgress,
MONGODB_ERROR_CODES.HostNotFound,
Expand All @@ -714,8 +735,8 @@ export function isRetryableError(error: MongoError): boolean {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
(typeof error.code === 'number' && RETRYABLE_ERROR_CODES.has(error.code!)) ||
error instanceof MongoNetworkError ||
!!error.message.match(/not master/) ||
!!error.message.match(/node is recovering/)
!!error.message.match(new RegExp(LEGACY_NOT_WRITABLE_PRIMARY_ERROR_MESSAGE)) ||
!!error.message.match(new RegExp(NODE_IS_RECOVERING_ERROR_MESSAGE))
);
}

Expand All @@ -724,12 +745,12 @@ const SDAM_RECOVERING_CODES = new Set<number>([
MONGODB_ERROR_CODES.PrimarySteppedDown,
MONGODB_ERROR_CODES.InterruptedAtShutdown,
MONGODB_ERROR_CODES.InterruptedDueToReplStateChange,
MONGODB_ERROR_CODES.NotMasterOrSecondary
MONGODB_ERROR_CODES.NotPrimaryOrSecondary
]);

const SDAM_NOTMASTER_CODES = new Set<number>([
MONGODB_ERROR_CODES.NotMaster,
MONGODB_ERROR_CODES.NotMasterNoSlaveOk,
const SDAM_NOTPRIMARY_CODES = new Set<number>([
MONGODB_ERROR_CODES.NotWritablePrimary,
MONGODB_ERROR_CODES.NotPrimaryNoSecondaryOk,
MONGODB_ERROR_CODES.LegacyNotPrimary
]);

Expand All @@ -744,20 +765,23 @@ function isRecoveringError(err: MongoError) {
return SDAM_RECOVERING_CODES.has(err.code);
}

return /not master or secondary/.test(err.message) || /node is recovering/.test(err.message);
return (
new RegExp(LEGACY_NOT_PRIMARY_OR_SECONDARY_ERROR_MESSAGE).test(err.message) ||
new RegExp(NODE_IS_RECOVERING_ERROR_MESSAGE).test(err.message)
);
}

function isNotMasterError(err: MongoError) {
function isNotWritablePrimaryError(err: MongoError) {
if (typeof err.code === 'number') {
// If any error code exists, we ignore the error.message
return SDAM_NOTMASTER_CODES.has(err.code);
return SDAM_NOTPRIMARY_CODES.has(err.code);
}

if (isRecoveringError(err)) {
return false;
}

return /not master/.test(err.message);
return new RegExp(LEGACY_NOT_WRITABLE_PRIMARY_ERROR_MESSAGE).test(err.message);
}

export function isNodeShuttingDownError(err: MongoError): boolean {
Expand All @@ -778,7 +802,7 @@ export function isSDAMUnrecoverableError(error: MongoError): boolean {
return true;
}

return isRecoveringError(error) || isNotMasterError(error);
return isRecoveringError(error) || isNotWritablePrimaryError(error);
}

export function isNetworkTimeoutError(err: MongoError): err is MongoNetworkError {
Expand Down
89 changes: 88 additions & 1 deletion test/unit/error.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,17 @@ const { ReplSetFixture } = require('../tools/common');
const { ns } = require('../../src/utils');
const { Topology } = require('../../src/sdam/topology');
const { MongoNetworkError, MongoWriteConcernError } = require('../../src/index');
const { isRetryableEndTransactionError } = require('../../src/error');
const {
LEGACY_NOT_WRITABLE_PRIMARY_ERROR_MESSAGE,
LEGACY_NOT_PRIMARY_OR_SECONDARY_ERROR_MESSAGE,
NODE_IS_RECOVERING_ERROR_MESSAGE
} = require('../../src/error');
const {
isRetryableEndTransactionError,
MongoParseError,
isSDAMUnrecoverableError,
MongoError
} = require('../../src/error');
const {
PoolClosedError: MongoPoolClosedError,
WaitQueueTimeoutError: MongoWaitQueueTimeoutError
Expand Down Expand Up @@ -65,6 +75,83 @@ describe('MongoErrors', () => {
});
});

describe('#isSDAMUnrecoverableError', function () {
context('when the error is a MongoParseError', function () {
it('returns true', function () {
const error = new MongoParseError('');
expect(isSDAMUnrecoverableError(error)).to.be.true;
});
});

context('when the error is null', function () {
it('returns true', function () {
expect(isSDAMUnrecoverableError(null)).to.be.true;
});
});

context('when the error has a "node is recovering" error code', function () {
it('returns true', function () {
const error = new MongoError('');
// Code for NotPrimaryOrSecondary
error.code = 13436;
expect(isSDAMUnrecoverableError(error)).to.be.true;
});
});

context('when the error has a "not writable primary" error code', function () {
it('returns true', function () {
const error = new MongoError('');
// Code for NotWritablePrimary
error.code = 10107;
expect(isSDAMUnrecoverableError(error)).to.be.true;
});
});

context(
'when the code is not a "node is recovering" error and not a "not writable primary" error',
function () {
it('returns false', function () {
// If the response includes an error code, it MUST be solely used to determine if error is a "node is recovering" or "not writable primary" error.
const error = new MongoError(NODE_IS_RECOVERING_ERROR_MESSAGE);
error.code = 555;
expect(isSDAMUnrecoverableError(error)).to.be.false;
});
}
);

context(
'when the error message contains the legacy "not primary" message and no error code is used',
function () {
it('returns true', function () {
const error = new MongoError(`this is ${LEGACY_NOT_WRITABLE_PRIMARY_ERROR_MESSAGE}.`);
expect(isSDAMUnrecoverableError(error)).to.be.true;
});
}
);

context(
'when the error message contains "node is recovering" and no error code is used',
function () {
it('returns true', function () {
const error = new MongoError(`the ${NODE_IS_RECOVERING_ERROR_MESSAGE} from an error`);
expect(isSDAMUnrecoverableError(error)).to.be.true;
});
}
);

context(
'when the error message contains the legacy "not primary or secondary" message and no error code is used',
function () {
it('returns true', function () {
const error = new MongoError(
`this is ${LEGACY_NOT_PRIMARY_OR_SECONDARY_ERROR_MESSAGE}, so we have a problem `
);
expect(isSDAMUnrecoverableError(error)).to.be.true;
});
}
);
});

describe('when MongoNetworkError is constructed', () => {
it('should only define beforeHandshake symbol if boolean option passed in', function () {
const errorWithOptionTrue = new MongoNetworkError('', { beforeHandshake: true });
Expand Down