Update region parsing logic (#326)

Author: satyakigh

src/featureFlag/FeatureFlagSupplier.ts

@@ -1,4 +1,6 @@
 import { LoggerFactory } from '../telemetry/LoggerFactory';
+import { ScopedTelemetry } from '../telemetry/ScopedTelemetry';
+import { Telemetry } from '../telemetry/TelemetryDecorator';
 import { Closeable } from '../utils/Closeable';
 import { toString } from '../utils/String';
 import { CompoundFeatureFlag } from './CombinedFeatureFlags';
@@ -17,6 +19,9 @@ import { FeatureFlag, TargetedFeatureFlag } from './FeatureFlagI';
 const log = LoggerFactory.getLogger('FeatureFlagSupplier');
 
 export class FeatureFlagSupplier implements Closeable {
+    @Telemetry()
+    private readonly telemetry!: ScopedTelemetry;
+
     private readonly _featureFlags = new Map<FeatureFlagConfigKey, FeatureFlag | DynamicFeatureFlag>();
     private readonly _targetedFeatureFlags = new Map<
         TargetedFeatureFlagConfigKey,
@@ -27,7 +32,7 @@ export class FeatureFlagSupplier implements Closeable {
         for (const [key, builder] of Object.entries(FeatureBuilders)) {
             const ff = new DynamicFeatureFlag(
                 key,
-                () => featureConfigSupplier(key, configSupplier, defaultConfig),
+                () => featureConfigSupplier(key, configSupplier, defaultConfig, this.telemetry),
                 builder,
             );
             this._featureFlags.set(key, ff);
@@ -36,7 +41,7 @@ export class FeatureFlagSupplier implements Closeable {
         for (const [key, builder] of Object.entries(TargetedFeatureBuilders)) {
             const ff = new DynamicTargetedFeatureFlag(
                 key,
-                () => featureConfigSupplier(key, configSupplier, defaultConfig),
+                () => featureConfigSupplier(key, configSupplier, defaultConfig, this.telemetry),
                 builder,
             );
             this._targetedFeatureFlags.set(key, ff);
@@ -77,10 +82,13 @@ function featureConfigSupplier(
     key: string,
     configSupplier: () => unknown,
     defaultConfig: () => unknown,
+    telemetry: ScopedTelemetry,
 ): FeatureFlagConfigType | undefined {
+    telemetry.count('used.config.default', 0);
     try {
         return FeatureFlagConfigSchema.parse(configSupplier()).features[key];
     } catch (err) {
+        telemetry.count('used.config.default', 1);
         log.warn(err, `Failed to parse feature flag config: \n${toString(configSupplier())}. Using defaults instead`);
         return FeatureFlagConfigSchema.parse(defaultConfig()).features[key];
     }

src/utils/Region.ts

@@ -1,6 +1,6 @@
 // https://docs.aws.amazon.com/AWSCloudFormation/latest/TemplateReference/resource-type-schemas.html
 import { LoggerFactory } from '../telemetry/LoggerFactory';
-import { dashesToUnderscores, toString } from './String';
+import { toString } from './String';
 
 // Make sure keys and values are exactly the same (ignore casing, '-', '_')
 export enum AwsRegion {
@@ -51,24 +51,22 @@ export enum AwsRegion {
     CN_NORTHWEST_1 = 'cn-northwest-1',
 }
 
-function isValidRegion(region: string) {
-    return region in AwsRegion;
-}
+const Regions: ReadonlyArray<string> = Object.values(AwsRegion);
 
 export function getRegion(region: unknown): AwsRegion {
-    let enumKey = String(region).trim();
-    if (!isValidRegion(enumKey)) {
-        enumKey = enumKey.toUpperCase();
-        if (!isValidRegion(enumKey)) {
-            enumKey = dashesToUnderscores(enumKey);
-        }
+    const key = String(region)
+        .replaceAll('_', '-')
+        .replaceAll(/[^a-zA-Z0-9-]/g, '')
+        .toLowerCase()
+        .trim();
+
+    if (key.length < 4 || key.length > 25) {
+        throw new Error(`Invalid region ${toString(region)} (${key})`);
     }
 
-    const value = AwsRegion[enumKey as keyof typeof AwsRegion];
-    if (!value) {
+    if (!Regions.includes(key)) {
         LoggerFactory.getLogger('Region').warn(`Unknown region ${toString(region)}`);
-        return enumKey.replaceAll('_', '-').toLowerCase() as AwsRegion;
     }
 
-    return value;
+    return key as AwsRegion;
 }

tst/unit/utils/Errors.test.ts

@@ -56,7 +56,7 @@ Error: Request cancelled for key: SendDocuments
     test('full stack', () => {
         expect(
             extractLocationFromStack(String.raw`
-Error: ENOENT: no such file or directory, scandir 'cfn-lsp/cloudformation-languageserver/bundle/development/.aws-cfn-storage/lmdb'
+Error: ENOENT: no such file or directory, scandir 'some-dir/cloudformation-languageserver/bundle/development/.aws-cfn-storage/lmdb'
     at readdirSync (node:fs:1584:26)
     at node:electron/js2c/node_init:2:16044
     at LMDBStoreFactory.cleanupOldVersions (webpack://aws/cloudformation-languageserver/src/datastore/LMDB.ts?d928:98:36)
@@ -66,7 +66,7 @@ Error: ENOENT: no such file or directory, scandir 'cfn-lsp/cloudformation-langua
 `),
         ).toEqual({
             'error.message':
-                "Error: ENOENT: no such file or directory, scandir 'cfn-lsp/cloudformation-languageserver/bundle/development/.aws-cfn-storage/lmdb'",
+                "Error: ENOENT: no such file or directory, scandir 'some-dir/cloudformation-languageserver/bundle/development/.aws-cfn-storage/lmdb'",
             'error.stack0': 'readdirSync node:fs 1584:26',
             'error.stack1': 'undefined node:electron/js2c/node_init 2:16044',
             'error.stack2':

tst/unit/utils/Region.test.ts

@@ -26,9 +26,24 @@ describe('Region', () => {
             expect(getRegion('EU_WEST_1')).toBe(AwsRegion.EU_WEST_1);
         });
 
-        it('should not throw when region is unknown or invalid', () => {
+        it('should not throw when region has a valid pattern', () => {
             expect(getRegion('invalid_region')).toBe('invalid-region');
             expect(getRegion('US_test-1')).toBe('us-test-1');
         });
+
+        it('can parse all regions', () => {
+            for (const reg of Object.keys(AwsRegion)) {
+                expect(getRegion(reg)).toBe(reg.toLowerCase().replaceAll('_', '-'));
+            }
+
+            for (const reg of Object.values(AwsRegion)) {
+                expect(getRegion(reg)).toBe(reg);
+            }
+        });
+
+        it('throws on invalid regions', () => {
+            expect(() => getRegion('a')).toThrow('Invalid region a (a)');
+            expect(() => getRegion('a'.repeat(26))).toThrow(`Invalid region ${'a'.repeat(26)} (${'a'.repeat(26)})`);
+        });
     });
 });