fix: command should not fail when global config fails to write

samcli/cli/global_config.py

@@ -307,7 +307,7 @@ def _load_config(self) -> None:
             for key in json_body:
                 self._persistent_fields.append(key)
         except (OSError, ValueError) as ex:
-            LOG.debug(
+            LOG.warning(
                 "Error when loading global config file: %s",
                 self.config_path,
                 exc_info=ex,
@@ -319,10 +319,17 @@ def _write_config(self) -> None:
         if not self._config_data:
             return
         config_data = {key: value for (key, value) in self._config_data.items() if key in self._persistent_fields}
-        json_str = json.dumps(config_data, indent=4)
-        if not self.config_dir.exists():
-            self.config_dir.mkdir(mode=0o700, parents=True, exist_ok=True)
-        self.config_path.write_text(json_str)
+        try:
+            json_str = json.dumps(config_data, indent=4)
+            if not self.config_dir.exists():
+                self.config_dir.mkdir(mode=0o700, parents=True, exist_ok=True)
+            self.config_path.write_text(json_str)
+        except (OSError, ValueError) as ex:
+            LOG.warning(
+                "Error when writing global config file: %s",
+                self.config_path,
+                exc_info=ex,
+            )
 
     @property
     def installation_id(self):

tests/functional/commands/cli/test_global_config.py

@@ -4,7 +4,7 @@
 import os
 from time import time
 
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, patch, ANY
 from unittest import TestCase
 from samcli.cli.global_config import GlobalConfig
 from pathlib import Path
@@ -237,7 +237,8 @@ def test_setter_on_invalid_json(self):
         gc.telemetry_enabled = True
         self.assertTrue(gc.telemetry_enabled)
 
-    def test_setter_cannot_open_file(self):
+    @patch("samcli.cli.global_config.LOG")
+    def test_setter_cannot_open_file(self, patched_logger):
         path = Path(self._cfg_dir, "metadata.json")
         with open(str(path), "w") as f:
             cfg = {"telemetryEnabled": True}
@@ -247,5 +248,5 @@ def test_setter_cannot_open_file(self):
         gc = GlobalConfig()
         gc.config_dir = Path(self._cfg_dir)
         with patch("samcli.cli.global_config.Path.write_text", m):
-            with self.assertRaises(OSError):
-                gc.telemetry_enabled = True
+            gc.telemetry_enabled = True
+            patched_logger.warning.assert_called_with("Error when writing global config file: %s", ANY, exc_info=ANY)

tests/unit/cli/test_global_config.py

@@ -1,5 +1,5 @@
 import os
-from unittest.mock import ANY, MagicMock, patch
+from unittest.mock import ANY, MagicMock, patch, mock_open
 from unittest import TestCase
 from samcli.cli.global_config import ConfigEntry, DefaultEntry, GlobalConfig
 from pathlib import Path
@@ -53,6 +53,26 @@ def tearDown(self):
         # Force singleton to recreate after each test
         GlobalConfig._Singleton__instance = None
 
+    def test_config_write_error(self):
+        self.path_write_mock.side_effect = IOError("fail")
+        gc = GlobalConfig()
+        installation_id = gc.installation_id
+        self.assertIsNotNone(installation_id)
+
+    def test_unable_to_create_dir(self):
+        self.path_exists_mock.return_value = False
+        self.path_mkdir_mock.side_effect = OSError("Permission DENIED")
+        gc = GlobalConfig()
+        installation_id = gc.installation_id
+        self.assertIsNotNone(installation_id)
+        telemetry_enabled = gc.telemetry_enabled
+        self.assertFalse(telemetry_enabled)
+
+    def test_setter_cannot_open_path(self):
+        self.path_read_mock.side_effect = IOError("fail")
+        gc = GlobalConfig()
+        gc.telemetry_enabled = True
+
     def test_singleton(self):
         gc1 = GlobalConfig()
         gc2 = GlobalConfig()
@@ -234,6 +254,16 @@ def test_write_config(self):
         self.path_mkdir_mock.assert_called_once()
         self.path_write_mock.assert_called_once()
 
+    def test_write_config_error(self):
+        self.path_exists_mock.return_value = False
+        GlobalConfig()._persistent_fields = ["a"]
+        GlobalConfig()._config_data = {"a": 1}
+        self.path_write_mock.side_effect = ValueError()
+        GlobalConfig()._write_config()
+        self.json_mock.dumps.assert_called_once()
+        self.path_mkdir_mock.assert_called_once()
+        self.path_write_mock.assert_called_once()
+
     @patch("samcli.cli.global_config.uuid.uuid4")
     def test_get_installation_id_saved(self, uuid_mock):
         self.json_mock.loads.return_value = {DefaultEntry.INSTALLATION_ID.config_key: "saved_uuid"}