fix #7475 feat(nimbus): Handle schema load failure (#7486)

* fix #7475 feat(nimbus): Add exception for json decode error

* fix #7475 test(nimbus): Test invalid remote json error

* pr feedback

- Move exception handling into Features class
- Add mocking to tests

* fix #7475 feat(nimbus): Update message

Co-authored-by: Jared Lockhart <[email protected]>

app/experimenter/features/__init__.py

@@ -53,7 +53,11 @@ class Feature(BaseModel):
     def load_remote_jsonschema(self):
         schema_url = urljoin(MC_ROOT, self.schema_paths.path)
         schema_data = requests.get(schema_url).content
-        return json.dumps(json.loads(schema_data), indent=2)
+
+        try:
+            return json.dumps(json.loads(schema_data), indent=2)
+        except json.JSONDecodeError:
+            return None
 
     def generate_jsonschema(self):
         schema = {

app/experimenter/features/management/commands/load_feature_configs.py

@@ -21,8 +21,11 @@ def handle(self, *args, **options):
             )
             feature_config.name = feature.slug
             feature_config.description = feature.description
-            feature_config.schema = feature.get_jsonschema()
             feature_config.read_only = True
+
+            if (schema := feature.get_jsonschema()) is not None:
+                feature_config.schema = schema
+
             feature_config.save()
             logger.info(f"Feature Loaded: {feature.applicationSlug}/{feature.slug}")
 

app/experimenter/features/tests/__init__.py

@@ -20,3 +20,11 @@
         pathlib.Path(__file__).parent.absolute(), "fixtures", "remote_schema_features"
     )
 )
+
+mock_invalid_remote_schema_features = override_settings(
+    FEATURE_MANIFESTS_PATH=os.path.join(
+        pathlib.Path(__file__).parent.absolute(),
+        "fixtures",
+        "invalid_remote_schema_features",
+    )
+)

app/experimenter/features/tests/fixtures/invalid_remote_schema_features/firefox-desktop.yaml

@@ -0,0 +1,9 @@
+cfr:
+  description: "Doorhanger message template for Messaging System"
+  hasExposure: true
+  exposureDescription: "Exposure is sent if the message is about to be shown after trigger and targeting conditions on the message matched."
+  isEarlyStartup: false
+  schema:
+    uri: "resource://activity-stream/schemas/CFR/ExtensionDoorhanger.invalidSchema.json"
+    path: "path/to/invalidSchema.json"
+  variables: {}

app/experimenter/features/tests/fixtures/remote_schema_features/firefox-desktop.yaml

@@ -2,7 +2,7 @@ cfr:
   description: "Doorhanger message template for Messaging System"
   hasExposure: true
   exposureDescription: "Exposure is sent if the message is about to be shown after trigger and targeting conditions on the message matched."
-  isEarlyStartup: false
+  isEarlyStartup: true
   schema:
     uri: "resource://activity-stream/schemas/CFR/ExtensionDoorhanger.schema.json"
     path: "path/to/schema.json"

app/experimenter/features/tests/test_features.py

@@ -195,14 +195,11 @@ def test_raises_request_error(self):
             )[0]
             desktop_feature.get_jsonschema()
 
-    def test_raises_json_error(self):
+    def test_returns_none_for_invalid_json(self):
         self.setup_json_error()
 
-        with self.assertRaises(json.JSONDecodeError):
-            desktop_feature = Features.by_application(
-                NimbusConstants.Application.DESKTOP
-            )[0]
-            desktop_feature.get_jsonschema()
+        desktop_feature = Features.by_application(NimbusConstants.Application.DESKTOP)[0]
+        self.assertIsNone(desktop_feature.get_jsonschema())
 
 
 class TestCheckFeatures(TestCase):

app/experimenter/features/tests/test_load_feature_configs.py

@@ -1,12 +1,17 @@
 import json
 
+import mock
 from django.core.management import call_command
 from django.test import TestCase
 
+from experimenter.experiments.constants import NimbusConstants
 from experimenter.experiments.models import NimbusExperiment, NimbusFeatureConfig
 from experimenter.experiments.tests.factories import NimbusFeatureConfigFactory
 from experimenter.features import Features
-from experimenter.features.tests import mock_valid_features
+from experimenter.features.tests import (
+    mock_invalid_remote_schema_features,
+    mock_valid_features,
+)
 
 
 @mock_valid_features
@@ -98,3 +103,32 @@ def test_handles_existing_features_with_same_slug_different_name(self):
             feature_config.description,
             "Some Firefox Feature",
         )
+
+
+@mock_invalid_remote_schema_features
+class TestLoadInvalidRemoteSchemaFeatureConfigs(TestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        Features.clear_cache()
+
+    def setUp(self):
+        self.remote_schema = "{invalid json"
+
+        mock_requests_get_patcher = mock.patch("experimenter.features.requests.get")
+        self.mock_requests_get = mock_requests_get_patcher.start()
+        self.addCleanup(mock_requests_get_patcher.stop)
+        mock_response = mock.MagicMock()
+        mock_response.content = self.remote_schema
+        self.mock_requests_get.return_value = mock_response
+
+    def test_load_feature_config_ignores_invalid_remote_json(self):
+        schema = "{}"
+        NimbusFeatureConfigFactory.create(
+            slug="cfr", application=NimbusConstants.Application.DESKTOP, schema=schema
+        )
+
+        call_command("load_feature_configs")
+
+        feature_config = NimbusFeatureConfig.objects.get(slug="cfr")
+        self.assertEqual(feature_config.schema, schema)