Clarify error messaging if 'isPubliclyHosted' key is missing (#6660)

flutter · Nov 3, 2023 · 4370128 · 4370128
1 parent d09ba71
commit 4370128
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 10 deletions.
diff --git a/packages/devtools_shared/lib/src/extensions/extension_model.dart b/packages/devtools_shared/lib/src/extensions/extension_model.dart
@@ -45,15 +45,34 @@ class DevToolsExtensionConfig implements Comparable {
         isPubliclyHosted: bool.parse(isPubliclyHosted),
       );
     } else {
-      const requiredKeys = {
+      if (!json.keys.contains(isPubliclyHostedKey)) {
+        throw StateError(
+          'Missing key "$isPubliclyHostedKey" when trying to parse '
+          'DevToolsExtensionConfig object.',
+        );
+      }
+
+      const requiredKeysFromConfigFile = {
         nameKey,
         pathKey,
         issueTrackerKey,
         versionKey,
-        isPubliclyHostedKey,
       };
-      final diff = requiredKeys.difference(json.keys.toSet());
-      if (diff.isEmpty) {
+      // We do not expect the config.yaml file to contain
+      // [isPubliclyHostedKey], as this should be inferred.
+      final jsonKeysFromConfigFile = Set.of(json.keys.toSet())
+        ..remove(isPubliclyHostedKey);
+
+      final diff = requiredKeysFromConfigFile.difference(
+        jsonKeysFromConfigFile,
+      );
+
+      if (diff.isNotEmpty) {
+        throw StateError(
+          'Missing required fields ${diff.toString()} in the extension '
+          'config.yaml.',
+        );
+      } else {
         // All the required keys are present, but the value types did not match.
         final sb = StringBuffer();
         for (final entry in json.entries) {
@@ -66,11 +85,6 @@ class DevToolsExtensionConfig implements Comparable {
           'values to be of type String, but one or more had a different type:\n'
           '${sb.toString()}',
         );
-      } else {
-        throw StateError(
-          'Missing required fields ${diff.toString()} in the extension '
-          'config.yaml.',
-        );
       }
     }
   }

diff --git a/packages/devtools_shared/test/extensions/extension_model_test.dart b/packages/devtools_shared/test/extensions/extension_model_test.dart
@@ -68,6 +68,16 @@ void main() {
         );
       }
 
+      Matcher throwsMissingIsPubliclyHostedError() {
+        return throwsA(
+          isA<StateError>().having(
+            (e) => e.message,
+            'missing isPubliclyHosted key StateError',
+            startsWith('Missing key "isPubliclyHosted"'),
+          ),
+        );
+      }
+
       // Missing 'name'.
       expect(
         () {
@@ -130,7 +140,7 @@ void main() {
             'issueTracker': 'www.google.com',
           });
         },
-        throwsMissingRequiredFieldsError(),
+        throwsMissingIsPubliclyHostedError(),
       );
     });