Skip to content

Commit 10abf03

Browse files
rename isSDAMUnrecoverableError to match spec's name
1 parent 49c5b6f commit 10abf03

File tree

3 files changed

+19
-30
lines changed

3 files changed

+19
-30
lines changed

src/error.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1528,13 +1528,7 @@ export function isNodeShuttingDownError(err: MongoError): boolean {
15281528
*
15291529
* @see https://github.com/mongodb/specifications/blob/master/source/server-discovery-and-monitoring/server-discovery-and-monitoring.md#not-writable-primary-and-node-is-recovering
15301530
*/
1531-
export function isSDAMUnrecoverableError(error: MongoError): boolean {
1532-
// NOTE: null check is here for a strictly pre-CMAP world, a timeout or
1533-
// close event are considered unrecoverable
1534-
if (error instanceof MongoParseError || error == null) {
1535-
return true;
1536-
}
1537-
1531+
export function isStateChangeError(error: MongoError): boolean {
15381532
return isRecoveringError(error) || isNotWritablePrimaryError(error);
15391533
}
15401534

src/sdam/server.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,13 @@ import {
2222
import {
2323
type AnyError,
2424
isNodeShuttingDownError,
25-
isSDAMUnrecoverableError,
25+
isStateChangeError,
2626
MONGODB_ERROR_CODES,
2727
MongoError,
2828
MongoErrorLabel,
2929
MongoNetworkError,
3030
MongoNetworkTimeoutError,
31+
MongoParseError,
3132
MongoRuntimeError,
3233
MongoServerClosedError,
3334
type MongoServerError,
@@ -412,7 +413,14 @@ export class Server extends TypedEventEmitter<ServerEvents> {
412413
this.pool.clear({ serviceId: connection.serviceId });
413414
}
414415
} else {
415-
if (isSDAMUnrecoverableError(error)) {
416+
// TODO: considering parse errors as SDAM unrecoverable errors seem
417+
// questionable. What if the parse error only comes from an application connection,
418+
// indicating some bytes were lost in transmission? It seems overkill to completely
419+
// kill the server.
420+
// Parse errors from monitoring connections are already handled because the
421+
// error would be wrapped in a ServerHeartbeatFailedEvent, which would mark the
422+
// server unknown and clear the pool. Can we remove this?
423+
if (isStateChangeError(error) || error instanceof MongoParseError) {
416424
if (shouldHandleStateChangeError(this, error)) {
417425
const shouldClearPool = isNodeShuttingDownError(error);
418426
if (this.loadBalanced && connection && shouldClearPool) {

test/unit/error.test.ts

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import * as importsFromErrorSrc from '../../src/error';
1010
import {
1111
isResumableError,
1212
isRetryableReadError,
13-
isSDAMUnrecoverableError,
13+
isStateChangeError,
1414
LEGACY_NOT_PRIMARY_OR_SECONDARY_ERROR_MESSAGE,
1515
LEGACY_NOT_WRITABLE_PRIMARY_ERROR_MESSAGE,
1616
MONGODB_ERROR_CODES,
@@ -211,26 +211,13 @@ describe('MongoErrors', () => {
211211
});
212212
});
213213

214-
describe('#isSDAMUnrecoverableError', function () {
215-
context('when the error is a MongoParseError', function () {
216-
it('returns true', function () {
217-
const error = new MongoParseError('');
218-
expect(isSDAMUnrecoverableError(error)).to.be.true;
219-
});
220-
});
221-
222-
context('when the error is null', function () {
223-
it('returns true', function () {
224-
expect(isSDAMUnrecoverableError(null)).to.be.true;
225-
});
226-
});
227-
214+
describe('#isStateChangeError', function () {
228215
context('when the error has a "node is recovering" error code', function () {
229216
it('returns true', function () {
230217
const error = new MongoError('');
231218
// Code for NotPrimaryOrSecondary
232219
error.code = 13436;
233-
expect(isSDAMUnrecoverableError(error)).to.be.true;
220+
expect(isStateChangeError(error)).to.be.true;
234221
});
235222
});
236223

@@ -239,7 +226,7 @@ describe('MongoErrors', () => {
239226
const error = new MongoError('');
240227
// Code for NotWritablePrimary
241228
error.code = 10107;
242-
expect(isSDAMUnrecoverableError(error)).to.be.true;
229+
expect(isStateChangeError(error)).to.be.true;
243230
});
244231
});
245232

@@ -250,7 +237,7 @@ describe('MongoErrors', () => {
250237
// 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.
251238
const error = new MongoError(NODE_IS_RECOVERING_ERROR_MESSAGE.source);
252239
error.code = 555;
253-
expect(isSDAMUnrecoverableError(error)).to.be.false;
240+
expect(isStateChangeError(error)).to.be.false;
254241
});
255242
}
256243
);
@@ -262,7 +249,7 @@ describe('MongoErrors', () => {
262249
const error = new MongoError(
263250
`this is ${LEGACY_NOT_WRITABLE_PRIMARY_ERROR_MESSAGE.source}.`
264251
);
265-
expect(isSDAMUnrecoverableError(error)).to.be.true;
252+
expect(isStateChangeError(error)).to.be.true;
266253
});
267254
}
268255
);
@@ -272,7 +259,7 @@ describe('MongoErrors', () => {
272259
function () {
273260
it('returns true', function () {
274261
const error = new MongoError(`the ${NODE_IS_RECOVERING_ERROR_MESSAGE} from an error`);
275-
expect(isSDAMUnrecoverableError(error)).to.be.true;
262+
expect(isStateChangeError(error)).to.be.true;
276263
});
277264
}
278265
);
@@ -284,7 +271,7 @@ describe('MongoErrors', () => {
284271
const error = new MongoError(
285272
`this is ${LEGACY_NOT_PRIMARY_OR_SECONDARY_ERROR_MESSAGE}, so we have a problem `
286273
);
287-
expect(isSDAMUnrecoverableError(error)).to.be.true;
274+
expect(isStateChangeError(error)).to.be.true;
288275
});
289276
}
290277
);

0 commit comments

Comments
 (0)