Skip to content

Commit 5a5f128

Browse files
fix(nimbus): catch FmlError when FmlClient fails to parse a manifest (#15019)
Because * The `FmlClient` constructor can raise `FmlError.ValidationError` when a feature manifest has validation errors (e.g., a variable with both `gecko-pref` and `default`, which are now mutually exclusive as of the latest application-services build) * `NimbusFmlLoader.fml_client()` did not catch this exception, so it propagated up and caused a 500 error on any page that calls `get_invalid_fields_errors()` (including the audience page during experiment creation) * This was discovered via failing Fenix integration tests on PR #15002 (application-services version bump) — all `FIREFOX_FENIX` UI tests failed with `TimeoutException` because the audience page returned a 500 This commit * Wraps the `FmlClient()` constructor in a `try/except FmlError` so invalid manifests are logged and skipped rather than crashing the page * Adds unit tests for the graceful degradation in both `fml_client()` and `get_fml_errors()` Fixes #15018
1 parent e9f8ccd commit 5a5f128

File tree

2 files changed

+45
-6
lines changed

2 files changed

+45
-6
lines changed

experimenter/experimenter/features/manifests/nimbus_fml_loader.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from typing import Optional
55

66
from django.conf import settings
7-
from nimbus_megazord.fml import FmlClient
7+
from nimbus_megazord.fml import FmlClient, FmlError
88

99
from experimenter.experiments.constants import NimbusConstants
1010
from experimenter.experiments.models import NimbusFeatureVersion
@@ -55,10 +55,16 @@ def fml_client(self, version: Optional[NimbusFeatureVersion] = None) -> FmlClien
5555
"""
5656
file_path = self.file_path(version)
5757
if file_path is not None:
58-
return FmlClient(
59-
str(file_path),
60-
self.channel,
61-
)
58+
try:
59+
return FmlClient(
60+
str(file_path),
61+
self.channel,
62+
)
63+
except FmlError:
64+
logger.exception(
65+
f"Nimbus FML Loader: FmlClient failed to parse manifest: {file_path}"
66+
)
67+
return None
6268
else:
6369
logger.error("Nimbus FML Loader: Failed to get FmlClient.")
6470
return None

experimenter/experimenter/features/tests/test_nimbus_fml_loader.py

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from unittest.mock import patch
33

44
from django.test import TestCase
5-
from nimbus_megazord.fml import FmlClient
5+
from nimbus_megazord.fml import FmlClient, FmlError
66

77
from experimenter.experiments.constants import NimbusConstants
88
from experimenter.features.manifests.nimbus_fml_loader import NimbusFmlLoader
@@ -210,3 +210,36 @@ def test_return_no_errors_for_invalid_application(
210210
self.assertEqual(loader.application, None)
211211
self.assertEqual(result, [])
212212
self.assertIn("Nimbus FML Loader: Invalid application", log.output[0])
213+
214+
@patch(
215+
"nimbus_megazord.fml.FmlClient.__init__",
216+
side_effect=FmlError("gecko-pref and default are mutually exclusive"),
217+
)
218+
@mock_fml_features
219+
def test_fml_client_returns_none_when_manifest_fails_to_parse(self, _mock_client):
220+
loader = self.create_loader()
221+
with self.assertLogs(level="ERROR") as log:
222+
result = loader.fml_client()
223+
self.assertIsNone(result)
224+
self.assertIn(
225+
"Nimbus FML Loader: FmlClient failed to parse manifest",
226+
log.output[0],
227+
)
228+
229+
@patch(
230+
"nimbus_megazord.fml.FmlClient.__init__",
231+
side_effect=FmlError("gecko-pref and default are mutually exclusive"),
232+
)
233+
@mock_fml_features
234+
def test_get_fml_errors_returns_empty_when_manifest_fails_to_parse(
235+
self, _mock_client
236+
):
237+
loader = self.create_loader()
238+
test_blob = json.dumps({"enabled": True})
239+
with self.assertLogs(level="ERROR") as log:
240+
result = loader.get_fml_errors(test_blob, "cookie-banners")
241+
self.assertEqual(result, [])
242+
self.assertIn(
243+
"Nimbus FML Loader: FmlClient failed to parse manifest",
244+
log.output[0],
245+
)

0 commit comments

Comments
 (0)