From fa4b01b5582b5402287cf1b3dbefdc6d4b01102e Mon Sep 17 00:00:00 2001
From: Matt Broadstone <mbroadst@mongodb.com>
Date: Fri, 24 Jan 2020 09:12:24 -0500
Subject: [PATCH] fix(server): non-timeout network errors transition to Unknown
 state

Per the SDAM rules in the error handling section, a non-timeout
error encountered during operation execution MUST transition the
server to an `Unknown` state and kick off the SDAM flow.

NODE-2435
---
 lib/core/error.js               |  7 ++++++-
 lib/core/sdam/server.js         |  7 +++++--
 lib/core/sdam/topology.js       |  3 +++
 test/unit/sdam/topology.test.js | 34 +++++++++++++++++++++++++++++++++
 4 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/lib/core/error.js b/lib/core/error.js
index 1854932017..38fbb379a3 100644
--- a/lib/core/error.js
+++ b/lib/core/error.js
@@ -251,6 +251,10 @@ function isSDAMUnrecoverableError(error) {
   return false;
 }
 
+function isNetworkTimeoutError(err) {
+  return err instanceof MongoNetworkError && err.message.match(/timed out/);
+}
+
 module.exports = {
   MongoError,
   MongoNetworkError,
@@ -261,5 +265,6 @@ module.exports = {
   mongoErrorContextSymbol,
   isRetryableError,
   isSDAMUnrecoverableError,
-  isNodeShuttingDownError
+  isNodeShuttingDownError,
+  isNetworkTimeoutError
 };
diff --git a/lib/core/sdam/server.js b/lib/core/sdam/server.js
index 209f048be1..d60082a525 100644
--- a/lib/core/sdam/server.js
+++ b/lib/core/sdam/server.js
@@ -13,6 +13,7 @@ const MongoNetworkError = require('../error').MongoNetworkError;
 const collationNotSupported = require('../utils').collationNotSupported;
 const debugOptions = require('../connection/utils').debugOptions;
 const isSDAMUnrecoverableError = require('../error').isSDAMUnrecoverableError;
+const isNetworkTimeoutError = require('../error').isNetworkTimeoutError;
 const makeStateMachine = require('../utils').makeStateMachine;
 const common = require('./common');
 
@@ -447,9 +448,11 @@ function makeOperationHandler(server, options, callback) {
         if (options && options.session) {
           options.session.serverSession.isDirty = true;
         }
-      }
 
-      if (isSDAMUnrecoverableError(err)) {
+        if (!isNetworkTimeoutError(err)) {
+          server.emit('error', err);
+        }
+      } else if (isSDAMUnrecoverableError(err)) {
         server.emit('error', err);
       }
     }
diff --git a/lib/core/sdam/topology.js b/lib/core/sdam/topology.js
index 3e1880ed4b..4fb4d607dc 100644
--- a/lib/core/sdam/topology.js
+++ b/lib/core/sdam/topology.js
@@ -795,6 +795,9 @@ function destroyServer(server, topology, options, callback) {
   options = options || {};
   LOCAL_SERVER_EVENTS.forEach(event => server.removeAllListeners(event));
 
+  // register a no-op for errors, we don't care now that we are destroying the server
+  server.on('error', () => {});
+
   server.destroy(options, () => {
     topology.emit(
       'serverClosed',
diff --git a/test/unit/sdam/topology.test.js b/test/unit/sdam/topology.test.js
index 4a11edd53c..b45146cb6a 100644
--- a/test/unit/sdam/topology.test.js
+++ b/test/unit/sdam/topology.test.js
@@ -192,5 +192,39 @@ describe('Topology (unit)', function() {
         });
       });
     });
+
+    it('should set server to unknown on non-timeout network error', function(done) {
+      mockServer.setMessageHandler(request => {
+        const doc = request.document;
+        if (doc.ismaster) {
+          request.reply(Object.assign({}, mock.DEFAULT_ISMASTER, { maxWireVersion: 9 }));
+        } else if (doc.insert) {
+          request.connection.destroy();
+        } else {
+          request.reply({ ok: 1 });
+        }
+      });
+
+      const topology = new Topology(mockServer.uri());
+      topology.connect(err => {
+        expect(err).to.not.exist;
+
+        topology.selectServer('primary', (err, server) => {
+          expect(err).to.not.exist;
+          this.defer(() => topology.close());
+
+          let serverError;
+          server.on('error', err => (serverError = err));
+
+          server.command('test.test', { insert: { a: 42 } }, (err, result) => {
+            expect(result).to.not.exist;
+            expect(err).to.exist;
+            expect(err).to.eql(serverError);
+            expect(server.description.type).to.equal('Unknown');
+            done();
+          });
+        });
+      });
+    });
   });
 });