From b21f637c76edfdcba77014a9a3957d3da006d6b0 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 30 Apr 2024 15:11:19 +0200
Subject: [PATCH] fix(shell-api): do not throw exceptions from
 enable/disableTelemetry() MONGOSH-1769

Adding exceptions to these in 2306b9c1996 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.
---
 packages/cli-repl/src/cli-repl.ts   |  3 ++-
 packages/e2e-tests/test/e2e.spec.ts | 22 ++++++++++++++++++++
 packages/shell-api/src/shell-api.ts | 32 ++++++++++++++++++-----------
 3 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts
index 8b88ceac9..c7dc094ff 100644
--- 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
index 601bc4df9..88f5cbfd7 100644
--- 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
index 1f3e63b8b..b66688271 100644
--- 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);
     }
   }