chore(service-provider-server): fail fast for doomed connect MONGOSH-705 (#801)

This was brought up as a nice way of erroring early in the driver sync
meeting today.

Author: addaleax

packages/cli-repl/test/e2e.spec.ts

@@ -880,5 +880,23 @@ describe('e2e', function() {
       });
     });
   });
+
+  describe('fail-fast connections', () => {
+    it('fails fast for ENOTFOUND errors', async() => {
+      const shell = TestShell.start({ args: [
+        'mongodb://' + 'verymuchnonexistentdomainname'.repeat(10) + '.mongodb.net/'
+      ] });
+      const result = await shell.waitForPromptOrExit();
+      expect(result).to.deep.equal({ state: 'exit', exitCode: 1 });
+    });
+
+    it('fails fast for ECONNREFUSED errors', async() => {
+      const shell = TestShell.start({ args: [
+        '--port', '0'
+      ] });
+      const result = await shell.waitForPromptOrExit();
+      expect(result).to.deep.equal({ state: 'exit', exitCode: 1 });
+    });
+  });
 });
 

packages/cli-repl/test/test-shell.ts

@@ -6,7 +6,7 @@ import path from 'path';
 import stripAnsi from 'strip-ansi';
 import { eventually } from './helpers';
 
-export type TestShellStartupResult = { state: 'prompt' | 'exit'; exitCode?: number | undefined };
+export type TestShellStartupResult = { state: 'prompt' } | { state: 'exit'; exitCode: number };
 type SignalType = ChildProcess extends { kill: (signal: infer T) => any } ? T : never;
 
 // Assume that prompt strings are those that end in '> ' but do not contain

packages/service-provider-core/src/fast-failure-connect.spec.ts

@@ -0,0 +1,37 @@
+import { expect } from 'chai';
+import { isFastFailureConnectionError } from './index';
+
+class MongoNetworkError extends Error {
+  constructor(msg: string) {
+    super(msg);
+    this.name = this.constructor.name;
+  }
+}
+class MongoError extends Error {
+  constructor(msg: string) {
+    super(msg);
+    this.name = this.constructor.name;
+  }
+}
+
+describe('isFastFailureConnectionError', function() {
+  it('returns true for ECONNREFUSED', function() {
+    expect(isFastFailureConnectionError(new MongoNetworkError('ECONNREFUSED'))).to.equal(true);
+  });
+
+  it('returns true for ENOTFOUND', function() {
+    expect(isFastFailureConnectionError(new MongoNetworkError('ENOTFOUND'))).to.equal(true);
+  });
+
+  it('returns true for ENETUNREACH', function() {
+    expect(isFastFailureConnectionError(new MongoNetworkError('ENETUNREACH'))).to.equal(true);
+  });
+
+  it('returns true when an API version is reuqired', function() {
+    expect(isFastFailureConnectionError(new MongoError('The apiVersion parameter is required'))).to.equal(true);
+  });
+
+  it('returns false for generic errors', function() {
+    expect(isFastFailureConnectionError(new Error('could not connect'))).to.equal(false);
+  });
+});

packages/service-provider-core/src/fast-failure-connect.ts

@@ -0,0 +1,13 @@
+// It probably makes sense to put this into its own package/repository once
+// other tools start using it.
+
+export function isFastFailureConnectionError(error: Error) {
+  switch (error.name) {
+    case 'MongoNetworkError':
+      return /\b(ECONNREFUSED|ENOTFOUND|ENETUNREACH)\b/.test(error.message);
+    case 'MongoError':
+      return /The apiVersion parameter is required/.test(error.message);
+    default:
+      return false;
+  }
+}

packages/service-provider-core/src/index.ts

@@ -26,6 +26,7 @@ import ShellAuthOptions from './shell-auth-options';
 import { ConnectionString } from './connection-string';
 export * from './all-transport-types';
 export * from './all-fle-types';
+import { isFastFailureConnectionError } from './fast-failure-connect';
 
 const bson = {
   ObjectId,
@@ -57,5 +58,6 @@ export {
   bson,
   bsonStringifiers,
   ConnectInfo,
-  ConnectionString
+  ConnectionString,
+  isFastFailureConnectionError
 };

packages/service-provider-server/src/cli-service-provider.spec.ts

@@ -2,9 +2,10 @@ import { CommonErrors } from '@mongosh/errors';
 import chai, { expect } from 'chai';
 import { Collection, Db, MongoClient } from 'mongodb';
 import sinonChai from 'sinon-chai';
-import sinon, { StubbedInstance, stubInterface } from 'ts-sinon';
+import sinon, { StubbedInstance, stubInterface, stubConstructor } from 'ts-sinon';
 import CliServiceProvider, { connectMongoClient } from './cli-service-provider';
 import { ConnectionString } from '@mongosh/service-provider-core';
+import { EventEmitter } from 'events';
 
 chai.use(sinonChai);
 
@@ -36,48 +37,50 @@ describe('CliServiceProvider', () => {
   let collectionStub: StubbedInstance<Collection>;
 
   describe('connectMongoClient', () => {
+    class FakeMongoClient extends EventEmitter {
+      connect() {}
+      db() {}
+      close() {}
+    }
+
     it('connects once when no AutoEncryption set', async() => {
       const uri = 'localhost:27017';
-      const mClientType = stubInterface<typeof MongoClient>();
-      const mClient = stubInterface<MongoClient>();
-      mClientType.connect.onFirstCall().resolves(mClient);
-      const result = await connectMongoClient(uri, {}, mClientType);
-      const calls = mClientType.connect.getCalls();
-      expect(calls.length).to.equal(1);
-      expect(calls[0].args).to.deep.equal([
-        uri, {}
-      ]);
+      const mClient = stubConstructor(FakeMongoClient);
+      const mClientType = sinon.stub().returns(mClient);
+      mClient.connect.onFirstCall().resolves(mClient);
+      const result = await connectMongoClient(uri, {}, mClientType as any);
+      expect(mClientType.getCalls()).to.have.lengthOf(1);
+      expect(mClientType.getCalls()[0].args).to.deep.equal([uri, {}]);
+      expect(mClient.connect.getCalls()).to.have.lengthOf(1);
       expect(result).to.equal(mClient);
     });
     it('connects once when bypassAutoEncryption is true', async() => {
       const uri = 'localhost:27017';
       const opts = { autoEncryption: { bypassAutoEncryption: true } };
-      const mClientType = stubInterface<typeof MongoClient>();
-      const mClient = stubInterface<MongoClient>();
-      mClientType.connect.onFirstCall().resolves(mClient);
-      const result = await connectMongoClient(uri, opts, mClientType);
-      const calls = mClientType.connect.getCalls();
-      expect(calls.length).to.equal(1);
-      expect(calls[0].args).to.deep.equal([
-        uri, opts
-      ]);
+      const mClient = stubConstructor(FakeMongoClient);
+      const mClientType = sinon.stub().returns(mClient);
+      mClient.connect.onFirstCall().resolves(mClient);
+      const result = await connectMongoClient(uri, opts, mClientType as any);
+      expect(mClientType.getCalls()).to.have.lengthOf(1);
+      expect(mClientType.getCalls()[0].args).to.deep.equal([uri, opts]);
+      expect(mClient.connect.getCalls()).to.have.lengthOf(1);
       expect(result).to.equal(mClient);
     });
     it('connects twice when bypassAutoEncryption is false and enterprise via modules', async() => {
       const uri = 'localhost:27017';
       const opts = { autoEncryption: { bypassAutoEncryption: false } };
-      const mClientType = stubInterface<typeof MongoClient>();
-      const mClientFirst = stubInterface<MongoClient>();
+      const mClientFirst = stubConstructor(FakeMongoClient);
+      const mClientSecond = stubConstructor(FakeMongoClient);
+      const mClientType = sinon.stub();
       const commandSpy = sinon.spy();
       mClientFirst.db.returns({ admin: () => ({ command: (...args) => {
         commandSpy(...args);
         return { modules: [ 'enterprise' ] };
       } } as any) } as any);
-      const mClientSecond = stubInterface<MongoClient>();
-      mClientType.connect.onFirstCall().resolves(mClientFirst);
-      mClientType.connect.onSecondCall().resolves(mClientSecond);
-      const result = await connectMongoClient(uri, opts, mClientType);
-      const calls = mClientType.connect.getCalls();
+      mClientType.onFirstCall().returns(mClientFirst);
+      mClientType.onSecondCall().returns(mClientSecond);
+      const result = await connectMongoClient(uri, opts, mClientType as any);
+      const calls = mClientType.getCalls();
       expect(calls.length).to.equal(2);
       expect(calls[0].args).to.deep.equal([
         uri, {}
@@ -88,18 +91,18 @@ describe('CliServiceProvider', () => {
     it('errors when bypassAutoEncryption is falsy and not enterprise', async() => {
       const uri = 'localhost:27017';
       const opts = { autoEncryption: {} };
-      const mClientType = stubInterface<typeof MongoClient>();
-      const mClientFirst = stubInterface<MongoClient>();
+      const mClientFirst = stubConstructor(FakeMongoClient);
+      const mClientSecond = stubConstructor(FakeMongoClient);
+      const mClientType = sinon.stub();
       const commandSpy = sinon.spy();
       mClientFirst.db.returns({ admin: () => ({ command: (...args) => {
         commandSpy(...args);
         return { modules: [] };
       } } as any) } as any);
-      const mClientSecond = stubInterface<MongoClient>();
-      mClientType.connect.onFirstCall().resolves(mClientFirst);
-      mClientType.connect.onSecondCall().resolves(mClientSecond);
+      mClientType.onFirstCall().returns(mClientFirst);
+      mClientType.onSecondCall().returns(mClientSecond);
       try {
-        await connectMongoClient(uri, opts, mClientType);
+        await connectMongoClient(uri, opts, mClientType as any);
       } catch (e) {
         return expect(e.message.toLowerCase()).to.include('automatic encryption');
       }
@@ -108,23 +111,47 @@ describe('CliServiceProvider', () => {
     it('errors when bypassAutoEncryption is falsy, missing modules', async() => {
       const uri = 'localhost:27017';
       const opts = { autoEncryption: {} };
-      const mClientType = stubInterface<typeof MongoClient>();
-      const mClientFirst = stubInterface<MongoClient>();
+      const mClientFirst = stubConstructor(FakeMongoClient);
+      const mClientSecond = stubConstructor(FakeMongoClient);
+      const mClientType = sinon.stub();
       const commandSpy = sinon.spy();
       mClientFirst.db.returns({ admin: () => ({ command: (...args) => {
         commandSpy(...args);
         return {};
       } } as any) } as any);
-      const mClientSecond = stubInterface<MongoClient>();
-      mClientType.connect.onFirstCall().resolves(mClientFirst);
-      mClientType.connect.onSecondCall().resolves(mClientSecond);
+      mClientType.onFirstCall().returns(mClientFirst);
+      mClientType.onSecondCall().returns(mClientSecond);
       try {
-        await connectMongoClient(uri, opts, mClientType);
+        await connectMongoClient(uri, opts, mClientType as any);
       } catch (e) {
         return expect(e.message.toLowerCase()).to.include('automatic encryption');
       }
       expect.fail('Failed to throw expected error');
     });
+
+    it('fails fast if there is a fail-fast connection error', async() => {
+      const err = Object.assign(new Error('ENOTFOUND'), { name: 'MongoNetworkError' });
+      const uri = 'localhost:27017';
+      const mClient = new FakeMongoClient();
+      const mClientType = sinon.stub().returns(mClient);
+      let rejectConnect;
+      mClient.close = sinon.stub().callsFake(() => {
+        rejectConnect(new Error('discarded error'));
+      });
+      mClient.connect = () => new Promise((resolve, reject) => {
+        rejectConnect = reject;
+        setImmediate(() => {
+          mClient.emit('serverHeartbeatFailed', { failure: err });
+        });
+      });
+      try {
+        await connectMongoClient(uri, {}, mClientType as any);
+      } catch (e) {
+        expect((mClient.close as any).getCalls()).to.have.lengthOf(1);
+        return expect(e).to.equal(err);
+      }
+      expect.fail('Failed to throw expected error');
+    });
   });
 
   describe('#constructor', () => {

packages/service-provider-server/src/cli-service-provider.ts

@@ -19,7 +19,8 @@ import {
   Topology,
   ReadPreferenceFromOptions,
   ReadPreferenceLike,
-  OperationOptions
+  OperationOptions,
+  ServerHeartbeatFailedEvent
 } from 'mongodb';
 
 import {
@@ -75,7 +76,8 @@ import {
   bson as BSON,
   ConnectionString,
   FLE,
-  AutoEncryptionOptions
+  AutoEncryptionOptions,
+  isFastFailureConnectionError
 } from '@mongosh/service-provider-core';
 
 import { MongoshCommandFailed, MongoshInternalError, MongoshRuntimeError } from '@mongosh/errors';
@@ -131,21 +133,45 @@ const DEFAULT_BASE_OPTIONS: OperationOptions = Object.freeze({
   serializeFunctions: true
 });
 
+/**
+ * Takes an unconnected MongoClient and connects it, but fails fast for certain
+ * errors.
+ */
+async function connectWithFailFast(client: MongoClient): Promise<void> {
+  let failFastErr;
+  const heartbeatFailureListener = ({ failure }: ServerHeartbeatFailedEvent) => {
+    if (isFastFailureConnectionError(failure)) {
+      failFastErr = failure;
+      client.close();
+    }
+  };
+
+  client.addListener('serverHeartbeatFailed', heartbeatFailureListener);
+  try {
+    await client.connect();
+  } catch (err) {
+    throw failFastErr || err;
+  } finally {
+    client.removeListener('serverHeartbeatFailed', heartbeatFailureListener);
+  }
+}
+
 /**
  * Connect a MongoClient. If AutoEncryption is requested, first connect without the encryption options and verify that
  * the connection is to an enterprise cluster. If not, then error, otherwise close the connection and reconnect with the
  * options the user initially specified. Provide the client class as an additional argument in order to test.
  * @param uri {String}
  * @param clientOptions {MongoClientOptions}
- * @param mClient {MongoClient}
+ * @param MClient {MongoClient}
  */
-export async function connectMongoClient(uri: string, clientOptions: MongoClientOptions, mClient = MongoClient): Promise<MongoClient> {
+export async function connectMongoClient(uri: string, clientOptions: MongoClientOptions, MClient = MongoClient): Promise<MongoClient> {
   if (clientOptions.autoEncryption !== undefined &&
     !clientOptions.autoEncryption.bypassAutoEncryption) {
     // connect first without autoEncryptionOptions
     const optionsWithoutFLE = { ...clientOptions };
     delete optionsWithoutFLE.autoEncryption;
-    const client = await mClient.connect(uri, optionsWithoutFLE);
+    const client = new MClient(uri, optionsWithoutFLE);
+    await connectWithFailFast(client);
     const buildInfo = await client.db('admin').admin().command({ buildInfo: 1 });
     if (
       !(buildInfo.modules?.includes('enterprise')) &&
@@ -156,7 +182,9 @@ export async function connectMongoClient(uri: string, clientOptions: MongoClient
     }
     await client.close();
   }
-  return mClient.connect(uri, clientOptions);
+  const client = new MClient(uri, clientOptions);
+  await connectWithFailFast(client);
+  return client;
 }
 
 /**