diff --git a/packages/cli-repl/test/e2e.spec.ts b/packages/cli-repl/test/e2e.spec.ts index 7ae4e1198e..1c2dd8aede 100644 --- a/packages/cli-repl/test/e2e.spec.ts +++ b/packages/cli-repl/test/e2e.spec.ts @@ -878,5 +878,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 }); + }); + }); }); diff --git a/packages/cli-repl/test/test-shell.ts b/packages/cli-repl/test/test-shell.ts index f25fa73f1a..f0ef3615ae 100644 --- a/packages/cli-repl/test/test-shell.ts +++ b/packages/cli-repl/test/test-shell.ts @@ -5,7 +5,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 diff --git a/packages/service-provider-core/src/fast-failure-connect.spec.ts b/packages/service-provider-core/src/fast-failure-connect.spec.ts new file mode 100644 index 0000000000..ee2e6d9ad8 --- /dev/null +++ b/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); + }); +}); diff --git a/packages/service-provider-core/src/fast-failure-connect.ts b/packages/service-provider-core/src/fast-failure-connect.ts new file mode 100644 index 0000000000..528c72bcec --- /dev/null +++ b/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; + } +} diff --git a/packages/service-provider-core/src/index.ts b/packages/service-provider-core/src/index.ts index 2fcb101785..c6b3d1cf82 100644 --- a/packages/service-provider-core/src/index.ts +++ b/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 }; diff --git a/packages/service-provider-server/src/cli-service-provider.spec.ts b/packages/service-provider-server/src/cli-service-provider.spec.ts index 028fd70492..846a9049f9 100644 --- a/packages/service-provider-server/src/cli-service-provider.spec.ts +++ b/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; describe('connectMongoClient', () => { + class FakeMongoClient extends EventEmitter { + connect() {} + db() {} + close() {} + } + it('connects once when no AutoEncryption set', async() => { const uri = 'localhost:27017'; - const mClientType = stubInterface(); - const mClient = stubInterface(); - 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(); - const mClient = stubInterface(); - 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(); - const mClientFirst = stubInterface(); + 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(); - 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(); - const mClientFirst = stubInterface(); + 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(); - 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(); - const mClientFirst = stubInterface(); + 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(); - 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', () => { diff --git a/packages/service-provider-server/src/cli-service-provider.ts b/packages/service-provider-server/src/cli-service-provider.ts index 39270359b0..75da52c2e7 100644 --- a/packages/service-provider-server/src/cli-service-provider.ts +++ b/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 { + 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 { +export async function connectMongoClient(uri: string, clientOptions: MongoClientOptions, MClient = MongoClient): Promise { 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; } /**