From 36ba0520cdb1f497502035c3a9f437b12f693372 Mon Sep 17 00:00:00 2001
From: Oliver Sanders <oliver.sanders@metoffice.gov.uk>
Date: Fri, 24 Jan 2025 13:51:54 +0000
Subject: [PATCH] broadcast: report invalid settings

* We already report invalid cycle points and namespaces.
* However, we have been relying on client-side validation for settings
  (which doesn't apply to the GraphQL mutation).
* This also raises the potential for inter-version compatibility issues
  going unreported.
* This commit explicitly handles invalid settings in the same way as
  invalid cycle points and namespaces so that they are reported back to
  the user.
* Closes the issue part of #6429.
---
 cylc/flow/broadcast_mgr.py     | 16 +++++++++----
 cylc/flow/network/resolvers.py | 41 +++++++++++++++++++---------------
 cylc/flow/network/schema.py    | 11 ---------
 3 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/cylc/flow/broadcast_mgr.py b/cylc/flow/broadcast_mgr.py
index b9329114302..265c66c1b34 100644
--- a/cylc/flow/broadcast_mgr.py
+++ b/cylc/flow/broadcast_mgr.py
@@ -276,6 +276,7 @@ def put_broadcast(
           bad_options is as described in the docstring for self.clear().
         """
         modified_settings = []
+        bad_settings = []
         bad_point_strings = []
         bad_namespaces = []
 
@@ -284,10 +285,15 @@ def put_broadcast(
                 # Coerce setting to cylc runtime object,
                 # i.e. str to  DurationFloat.
                 coerced_setting = deepcopy(setting)
-                BroadcastConfigValidator().validate(
-                    coerced_setting,
-                    SPEC['runtime']['__MANY__'],
-                )
+                try:
+                    BroadcastConfigValidator().validate(
+                        coerced_setting,
+                        SPEC['runtime']['__MANY__'],
+                    )
+                except Exception as exc:
+                    LOG.error(exc)
+                    bad_settings.append(setting)
+                    continue
 
                 for point_string in point_strings or []:
                     # Standardise the point and check its validity.
@@ -324,6 +330,8 @@ def put_broadcast(
         LOG.info(get_broadcast_change_report(modified_settings))
 
         bad_options = {}
+        if bad_settings:
+            bad_options["settings"] = bad_settings
         if bad_point_strings:
             bad_options["point_strings"] = bad_point_strings
         if bad_namespaces:
diff --git a/cylc/flow/network/resolvers.py b/cylc/flow/network/resolvers.py
index 79b91f97ee7..65f96aaa55c 100644
--- a/cylc/flow/network/resolvers.py
+++ b/cylc/flow/network/resolvers.py
@@ -787,24 +787,29 @@ def broadcast(
         cutoff: Any = None
     ):
         """Put or clear broadcasts."""
-        if settings is not None:
-            # Convert schema field names to workflow config setting names if
-            # applicable:
-            for i, dict_ in enumerate(settings):
-                settings[i] = runtime_schema_to_cfg(dict_)
-
-        if mode == 'put_broadcast':
-            return self.schd.task_events_mgr.broadcast_mgr.put_broadcast(
-                cycle_points, namespaces, settings)
-        if mode == 'clear_broadcast':
-            return self.schd.task_events_mgr.broadcast_mgr.clear_broadcast(
-                point_strings=cycle_points,
-                namespaces=namespaces,
-                cancel_settings=settings)
-        if mode == 'expire_broadcast':
-            return self.schd.task_events_mgr.broadcast_mgr.expire_broadcast(
-                cutoff)
-        raise ValueError('Unsupported broadcast mode')
+        try:
+            if settings is not None:
+                # Convert schema field names to workflow config setting names if
+                # applicable:
+                for i, dict_ in enumerate(settings):
+                    settings[i] = runtime_schema_to_cfg(dict_)
+
+            if mode == 'put_broadcast':
+                return self.schd.task_events_mgr.broadcast_mgr.put_broadcast(
+                    cycle_points, namespaces, settings)
+            if mode == 'clear_broadcast':
+                return self.schd.task_events_mgr.broadcast_mgr.clear_broadcast(
+                    point_strings=cycle_points,
+                    namespaces=namespaces,
+                    cancel_settings=settings)
+            if mode == 'expire_broadcast':
+                return self.schd.task_events_mgr.broadcast_mgr.expire_broadcast(
+                    cutoff)
+            raise ValueError('Unsupported broadcast mode')
+        except Exception as exc:
+            LOG.error(exc)
+            return {'error': {'message': str(exc)}}
+
 
     def put_ext_trigger(
         self,
diff --git a/cylc/flow/network/schema.py b/cylc/flow/network/schema.py
index 67d3c58ec84..80b40b42d82 100644
--- a/cylc/flow/network/schema.py
+++ b/cylc/flow/network/schema.py
@@ -1778,17 +1778,6 @@ class Arguments:
             )
         )
 
-        # TODO: work out how to implement this feature, it needs to be
-        #       handled client-side which makes it slightly awkward in
-        #       api-on-the-fly land
-
-        # files = graphene.List(
-        #    String,
-        #    description=sstrip('''
-        #        File with config to broadcast. Can be used multiple times
-        #    ''')
-        # )
-
     result = GenericScalar()