From 265fe40cf29992764d1ab030a1ee4dca97cd7c7c Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Wed, 5 Feb 2020 08:40:56 -0500 Subject: [PATCH] fix: only consider MongoError subclasses for retryability A change to use the `RetryableWriteError` label to determine if a write should be retried ignored that non-MongoError's could also be caught. These should be ignored for the purposes of retryability --- lib/core/sdam/server.js | 3 ++- lib/core/sdam/topology.js | 9 ++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/core/sdam/server.js b/lib/core/sdam/server.js index df346e2d94..4a4ee864b1 100644 --- a/lib/core/sdam/server.js +++ b/lib/core/sdam/server.js @@ -16,6 +16,7 @@ const isSDAMUnrecoverableError = require('../error').isSDAMUnrecoverableError; const isNetworkTimeoutError = require('../error').isNetworkTimeoutError; const isRetryableWriteError = require('../error').isRetryableWriteError; const makeStateMachine = require('../utils').makeStateMachine; +const maxWireVersion = require('../utils').maxWireVersion; const common = require('./common'); const ServerType = common.ServerType; @@ -468,7 +469,7 @@ function makeOperationHandler(server, options, callback) { } } else { // if pre-4.4 server, then add error label if its a retryable write error - if (server.description.maxWireVersion < 9 && isRetryableWriteError(err)) { + if (maxWireVersion(server) < 9 && isRetryableWriteError(err)) { err.addErrorLabel('RetryableWriteError'); } diff --git a/lib/core/sdam/topology.js b/lib/core/sdam/topology.js index 978b50c225..418effd07c 100644 --- a/lib/core/sdam/topology.js +++ b/lib/core/sdam/topology.js @@ -14,7 +14,6 @@ const CoreCursor = require('../cursor').CoreCursor; const deprecate = require('util').deprecate; const BSON = require('../connection/utils').retrieveBSON(); const createCompressionInfo = require('../topologies/shared').createCompressionInfo; -const isRetryableError = require('../error').isRetryableError; const isNodeShuttingDownError = require('../error').isNodeShuttingDownError; const maxWireVersion = require('../utils').maxWireVersion; const ClientSession = require('../sessions').ClientSession; @@ -680,7 +679,7 @@ class Topology extends EventEmitter { const cb = (err, result) => { if (!err) return callback(null, result); - if (!err.hasErrorLabel('RetryableWriteError')) { + if (!shouldRetryOperation(err)) { return callback(err); } @@ -944,7 +943,7 @@ function executeWriteOperation(args, options, callback) { const handler = (err, result) => { if (!err) return callback(null, result); - if (!err.hasErrorLabel('RetryableWriteError')) { + if (!shouldRetryOperation(err)) { err = getMMAPError(err); return callback(err); } @@ -972,6 +971,10 @@ function executeWriteOperation(args, options, callback) { }); } +function shouldRetryOperation(err) { + return err instanceof MongoError && err.hasErrorLabel('RetryableWriteError'); +} + /** * Resets the internal state of this server to `Unknown` by simulating an empty ismaster *