fix(shell-api): do not throw exceptions from enable/disableTelemetry() …

…MONGOSH-1769 (#1966) Adding exceptions to these in 2306b9c was a breaking change for programmatic usage of mongosh, so we should revert this behavior (and possibly reconsider in a major version) while keeping the message itself as introduced in that commit.
mongodb-js · May 2, 2024 · f515be6 · f515be6
1 parent 64b41af
commit f515be6
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 13 deletions.
diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts
@@ -225,7 +225,8 @@ export class CliRepl implements MongoshIOProvider {
   get forceDisableTelemetry(): boolean {
     return (
       this.globalConfig?.forceDisableTelemetry ||
-      (this.isContainerizedEnvironment && !this.mongoshRepl.isInteractive)
+      (this.isContainerizedEnvironment && !this.mongoshRepl.isInteractive) ||
+      !!process.env.MONGOSH_FORCE_DISABLE_TELEMETRY_FOR_TESTING
     );
   }
 

diff --git a/packages/e2e-tests/test/e2e.spec.ts b/packages/e2e-tests/test/e2e.spec.ts
@@ -1446,6 +1446,28 @@ describe('e2e', function () {
           );
           expect((await readConfig()).enableTelemetry).to.equal(false);
         });
+        it('enableTelemetry() returns an error if forceDisableTelemetry is set (but does not throw)', async function () {
+          await shell.executeLine(
+            'process.env.MONGOSH_FORCE_DISABLE_TELEMETRY_FOR_TESTING = 1'
+          );
+          expect(
+            await shell.executeLine('enableTelemetry() + "<<<<"')
+          ).to.include(
+            "Cannot modify telemetry settings while 'forceDisableTelemetry' is set to true<<<<"
+          );
+          expect((await readConfig()).enableTelemetry).to.equal(true);
+        });
+        it('disableTelemetry() returns an error if forceDisableTelemetry is set (but does not throw)', async function () {
+          await shell.executeLine(
+            'process.env.MONGOSH_FORCE_DISABLE_TELEMETRY_FOR_TESTING = 1'
+          );
+          expect(
+            await shell.executeLine('disableTelemetry() + "<<<<"')
+          ).to.include(
+            "Cannot modify telemetry settings while 'forceDisableTelemetry' is set to true<<<<"
+          );
+          expect((await readConfig()).enableTelemetry).to.equal(true);
+        });
       });
 
       describe('log file', function () {

diff --git a/packages/shell-api/src/shell-api.ts b/packages/shell-api/src/shell-api.ts
@@ -332,24 +332,32 @@ export default class ShellApi extends ShellApiClass {
   @returnsPromise
   @platforms(['CLI'])
   async enableTelemetry(): Promise<any> {
-    const result = await this._instanceState.evaluationListener.setConfig?.(
-      'enableTelemetry',
-      true
-    );
-    if (result === 'success') {
-      return i18n.__('cli-repl.cli-repl.enabledTelemetry');
+    try {
+      const result = await this._instanceState.evaluationListener.setConfig?.(
+        'enableTelemetry',
+        true
+      );
+      if (result === 'success') {
+        return i18n.__('cli-repl.cli-repl.enabledTelemetry');
+      }
+    } catch (err: unknown) {
+      return String(err);
     }
   }
 
   @returnsPromise
   @platforms(['CLI'])
   async disableTelemetry(): Promise<any> {
-    const result = await this._instanceState.evaluationListener.setConfig?.(
-      'enableTelemetry',
-      false
-    );
-    if (result === 'success') {
-      return i18n.__('cli-repl.cli-repl.disabledTelemetry');
+    try {
+      const result = await this._instanceState.evaluationListener.setConfig?.(
+        'enableTelemetry',
+        false
+      );
+      if (result === 'success') {
+        return i18n.__('cli-repl.cli-repl.disabledTelemetry');
+      }
+    } catch (err: unknown) {
+      return String(err);
     }
   }