Просмотр исходного кода

fix(inbound-filters): Use string options instead of bools for react hydration errors filter (#57761)

Riccardo Busetti 1 год назад
Родитель
Сommit
c63d5be706

+ 1 - 1
src/sentry/api/endpoints/project_details.py

@@ -806,7 +806,7 @@ class ProjectDetailsEndpoint(ProjectEndpoint):
             if "filters:react-hydration-errors" in options:
                 project.update_option(
                     "filters:react-hydration-errors",
-                    bool(options["filters:react-hydration-errors"]),
+                    "1" if bool(options["filters:react-hydration-errors"]) else "0",
                 )
             if "filters:chunk-load-error" in options:
                 project.update_option(

+ 5 - 1
src/sentry/api/serializers/models/project.py

@@ -212,7 +212,11 @@ def format_options(attrs: dict[str, Any]) -> dict[str, Any]:
         ),
         "sentry:reprocessing_active": bool(options.get("sentry:reprocessing_active", False)),
         "filters:blacklisted_ips": "\n".join(options.get("sentry:blacklisted_ips", [])),
-        "filters:react-hydration-errors": bool(options.get("filters:react-hydration-errors", True)),
+        # This option was defaulted to string but was changed at runtime to a boolean due to an error in the
+        # implementation. In order to bring it back to a string, we need to repair on read stored options. This is
+        # why the value true is determined by either "1" or True.
+        "filters:react-hydration-errors": options.get("filters:react-hydration-errors", "1")
+        in ("1", True),
         "filters:chunk-load-error": options.get("filters:chunk-load-error", "1") == "1",
         f"filters:{FilterTypes.RELEASES}": "\n".join(
             options.get(f"sentry:{FilterTypes.RELEASES}", [])

+ 4 - 3
src/sentry/relay/config/__init__.py

@@ -136,9 +136,10 @@ def get_filter_settings(project: Project) -> Mapping[str, Any]:
 
         error_messages += project.get_option(f"sentry:{FilterTypes.ERROR_MESSAGES}") or []
 
-    # TODO: refactor the system to allow management of error messages filtering via the inbound filters, since right
-    #  now the system maps an option to a single top-level filter like "ignoreTransactions".
-    enable_react = project.get_option("filters:react-hydration-errors")
+    # This option was defaulted to string but was changed at runtime to a boolean due to an error in the
+    # implementation. In order to bring it back to a string, we need to repair on read stored options. This is
+    # why the value true is determined by either "1" or True.
+    enable_react = project.get_option("filters:react-hydration-errors") in ("1", True)
     if enable_react:
         # 418 - Hydration failed because the initial UI does not match what was rendered on the server.
         # 419 - The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.

+ 8 - 4
tests/sentry/api/endpoints/test_project_details.py

@@ -849,11 +849,15 @@ class ProjectUpdateTest(APITestCase):
         assert b"storeCrashReports" in resp.content
 
     def test_react_hydration_errors(self):
-        value = False
-        options = {"filters:react-hydration-errors": value}
+        options = {"filters:react-hydration-errors": False}
         resp = self.get_success_response(self.org_slug, self.proj_slug, options=options)
-        assert self.project.get_option("filters:react-hydration-errors") == value
-        assert resp.data["options"]["filters:react-hydration-errors"] == value
+        assert self.project.get_option("filters:react-hydration-errors") == "0"
+        assert resp.data["options"]["filters:react-hydration-errors"] is False
+
+        options = {"filters:react-hydration-errors": True}
+        resp = self.get_success_response(self.org_slug, self.proj_slug, options=options)
+        assert self.project.get_option("filters:react-hydration-errors") == "1"
+        assert resp.data["options"]["filters:react-hydration-errors"] is True
 
     def test_chunk_load_error(self):
         options = {"filters:chunk-load-error": False}

+ 20 - 4
tests/sentry/relay/test_config.py

@@ -178,7 +178,7 @@ def test_project_config_uses_filter_features(
     blacklisted_ips = ["112.69.248.54"]
     default_project.update_option("sentry:error_messages", error_messages)
     default_project.update_option("sentry:releases", releases)
-    default_project.update_option("filters:react-hydration-errors", False)
+    default_project.update_option("filters:react-hydration-errors", "0")
     default_project.update_option("filters:chunk-load-error", "0")
 
     if has_blacklisted_ips:
@@ -206,12 +206,28 @@ def test_project_config_uses_filter_features(
         assert cfg_client_ips is None
 
 
+@django_db_all
+@region_silo_test(stable=True)
+def test_project_config_with_react_hydration_errors_filter(default_project):
+    default_project.update_option("filters:chunk-load-error", "0")
+
+    # We want to test both string and boolean option representations. See implementation for more details
+    # on this.
+    for value in ("1", True):
+        default_project.update_option("filters:react-hydration-errors", value)
+        project_cfg = get_project_config(default_project, full_config=True)
+
+        cfg = project_cfg.to_dict()
+        _validate_project_config(cfg["config"])
+        cfg_error_messages = get_path(cfg, "config", "filterSettings", "errorMessages")
+
+        assert len(cfg_error_messages) == 1
+
+
 @django_db_all
 @region_silo_test(stable=True)
 def test_project_config_with_chunk_load_error_filter(default_project):
-    # The react-hydration-errors option is set as string in the defaults but then its changed as a boolean in the
-    # options endpoint, which is something that should be changed.
-    default_project.update_option("filters:react-hydration-errors", False)
+    default_project.update_option("filters:react-hydration-errors", "0")
     default_project.update_option("filters:chunk-load-error", "1")
 
     project_cfg = get_project_config(default_project, full_config=True)