SAM: change config to "machine" scope; handle failed config write

package.json

@@ -88,6 +88,7 @@
                 },
                 "aws.samcli.location": {
                     "type": "string",
+                    "scope": "machine",
                     "default": "",
                     "markdownDescription": "%AWS.configuration.description.samcli.location%"
                 },

src/shared/sam/cli/samCliConfiguration.ts

@@ -55,7 +55,7 @@ export class DefaultSamCliConfiguration implements SamCliConfiguration {
         const detectedLocation = (await this._samCliLocationProvider.getLocation()) ?? ''
         // Avoid setting the value redundantly (could cause a loop because we
         // listen to the `onDidChangeConfiguration` event).
-        if (configLocation !== detectedLocation) {
+        if (detectedLocation && configLocation !== detectedLocation) {
             await this.setSamCliLocation(detectedLocation)
         }
     }

src/shared/settingsConfiguration.ts

@@ -4,24 +4,39 @@
  */
 
 import * as vscode from 'vscode'
+import { getLogger } from './logger'
 
-// defines helper methods for interacting with VSCode's configuration
-// persistence mechanisms, allowing us to test with mocks.
+/**
+ * Wraps the VSCode configuration API and provides Toolkit-related
+ * configuration functions.
+ */
 export interface SettingsConfiguration {
     readSetting<T>(settingKey: string): T | undefined
     readSetting<T>(settingKey: string, defaultValue: T): T
 
     // array values are serialized as a comma-delimited string
-    writeSetting<T>(settingKey: string, value: T | undefined, target: vscode.ConfigurationTarget): Promise<void>
+    /**
+     * Sets a config value.
+     *
+     * Writing to the (VSCode) config store may fail if the user does not have
+     * write permissions, or if some requirement is not met.  For example, the
+     * `vscode.ConfigurationTarget.Workspace` scope requires a workspace.
+     *
+     * @param settingKey  Config key name
+     * @param value  Config value
+     * @param target  Config _scope_
+     *
+     * @returns true on success, else false
+     */
+    writeSetting<T>(settingKey: string, value: T | undefined, target: vscode.ConfigurationTarget): Promise<boolean>
 }
 
 // default configuration settings handler for production release
 export class DefaultSettingsConfiguration implements SettingsConfiguration {
     public constructor(public readonly extensionSettingsPrefix: string) {}
 
     public readSetting<T>(settingKey: string, defaultValue?: T): T | undefined {
-        // tslint:disable-next-line:no-null-keyword
-        const settings = vscode.workspace.getConfiguration(this.extensionSettingsPrefix, null)
+        const settings = vscode.workspace.getConfiguration(this.extensionSettingsPrefix)
 
         if (!settings) {
             return defaultValue
@@ -31,10 +46,14 @@ export class DefaultSettingsConfiguration implements SettingsConfiguration {
         return val ?? defaultValue
     }
 
-    public async writeSetting<T>(settingKey: string, value: T, target: vscode.ConfigurationTarget): Promise<void> {
-        // tslint:disable-next-line:no-null-keyword
-        const settings = vscode.workspace.getConfiguration(this.extensionSettingsPrefix, null)
-
-        await settings.update(settingKey, value, target)
+    public async writeSetting<T>(settingKey: string, value: T, target: vscode.ConfigurationTarget): Promise<boolean> {
+        try {
+            const settings = vscode.workspace.getConfiguration(this.extensionSettingsPrefix)
+            await settings.update(settingKey, value, target)
+            return true
+        } catch (e) {
+            getLogger().error('failed to set config: %O=%O, error: %O', settingKey, value, e)
+            return false
+        }
     }
 }

src/test/utilities/testSettingsConfiguration.ts

@@ -15,7 +15,8 @@ export class TestSettingsConfiguration implements SettingsConfiguration {
         return this._data[settingKey] as T
     }
 
-    public async writeSetting<T>(settingKey: string, value: T, target?: any): Promise<void> {
+    public async writeSetting<T>(settingKey: string, value: T, target?: any): Promise<boolean> {
         this._data[settingKey] = value
+        return true
     }
 }