CR comments

Author: aherlihy

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

@@ -100,7 +100,27 @@ describe('CliServiceProvider', () => {
       try {
         await connectMongoClient(uri, opts, mClientType);
       } catch (e) {
-        return expect(e.message).to.include('automatic encryption');
+        return expect(e.message.toLowerCase()).to.include('automatic encryption');
+      }
+      expect.fail('Failed to throw expected error');
+    });
+    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 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);
+      try {
+        await connectMongoClient(uri, opts, mClientType);
+      } catch (e) {
+        return expect(e.message.toLowerCase()).to.include('automatic encryption');
       }
       expect.fail('Failed to throw expected error');
     });

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

@@ -126,7 +126,7 @@ const DEFAULT_BASE_OPTIONS = Object.freeze({
  * @param clientOptions {MongoClientOptions}
  * @param mClient {MongoClient}
  */
-export async function connectMongoClient(uri: string, clientOptions: MongoClientOptions, mClient = MongoClient): Promise<MongoClient | void> {
+export async function connectMongoClient(uri: string, clientOptions: MongoClientOptions, mClient = MongoClient): Promise<MongoClient> {
   if (clientOptions.autoEncryption !== undefined &&
     !clientOptions.autoEncryption.bypassAutoEncryption) {
     // connect first without autoEncryptionOptions
@@ -135,11 +135,11 @@ export async function connectMongoClient(uri: string, clientOptions: MongoClient
     const client = await mClient.connect(uri, optionsWithoutFLE);
     const buildInfo = await client.db('admin').admin().command({ buildInfo: 1 });
     if (
-      !(buildInfo.gitVersion && buildInfo.gitVersion.match(/enterprise/)) &&
-      !(buildInfo.modules && buildInfo.modules.indexOf('enterprise') !== -1)
+      !(buildInfo.modules?.includes('enterprise')) &&
+      !(buildInfo.gitVersion?.match(/enterprise/))
     ) {
       await client.close();
-      throw new MongoshRuntimeError('Cannot turn on automatic encryption for connections to non-enterprise hosts');
+      throw new MongoshRuntimeError('Automatic encryption is only available with Atlas and MongoDB Enterprise');
     }
     await client.close();
   }
@@ -173,7 +173,7 @@ class CliServiceProvider extends ServiceProviderCore implements ServiceProvider
       await connectMongoClient(
         uri,
         clientOptions
-      ) as MongoClient :
+      ) :
       new MongoClient(uri || 'mongodb://nodb/', clientOptions);
 
     return new CliServiceProvider(mongoClient, clientOptions, uri);
@@ -223,7 +223,7 @@ class CliServiceProvider extends ServiceProviderCore implements ServiceProvider
     const mongoClient = await connectMongoClient(
       uri,
       clientOptions
-    ) as MongoClient;
+    );
     return new CliServiceProvider(mongoClient, uri);
   }
 
@@ -1054,7 +1054,7 @@ class CliServiceProvider extends ServiceProviderCore implements ServiceProvider
     const mc = await connectMongoClient(
       this.uri as string,
       clientOptions
-    ) as MongoClient;
+    );
     try {
       await this.mongoClient.close();
       // eslint-disable-next-line no-empty
@@ -1123,7 +1123,7 @@ class CliServiceProvider extends ServiceProviderCore implements ServiceProvider
     const mc = await connectMongoClient(
       this.uri as string,
       clientOptions
-    ) as MongoClient;
+    );
     try {
       await this.mongoClient.close();
       // eslint-disable-next-line no-empty