fix(gateway): consistent parameter-config error surface (list vs get) and drop dead classify fallback

docs/api/rest.rst

@@ -2530,6 +2530,42 @@ response), the response includes vendor metadata indicating partial results:
 When all peers respond successfully, these fields are omitted. See the
 :doc:`aggregation configuration guide </config/aggregation>` for setup details.
 
+``GET /{entity}/configurations`` applies the same honesty to its *local*
+backing nodes. This only arises for entities backed by more than one ROS 2
+node; a single-node entity has no partial state, since its one node either
+answers or the whole request fails. When several backing nodes contribute and
+some answer while others do not, the response stays ``200`` but flags itself
+``partial`` and names the failed nodes: nodes that were down or unresponsive
+(``503``-class) in ``unavailable_nodes``, any other per-node failure in
+``failed_nodes``. The parameters of the nodes that *did* answer are still
+returned - a partial list never discards them:
+
+.. code-block:: json
+
+   {
+     "items": [
+       {"id": "use_sim_time", "name": "use_sim_time", "type": "parameter"}
+     ],
+     "x-medkit": {
+       "partial": true,
+       "unavailable_nodes": ["/down_node"]
+     }
+   }
+
+This matches ``GET /{entity}/configurations/{param}``, which returns ``503``
+``x-medkit-ros2-node-unavailable`` for the same backing-node outage.
+
+When every backing node responds, ``unavailable_nodes`` and ``failed_nodes`` are
+omitted. Note that ``partial`` may still appear on its own from the peer
+fan-out described above, without any local unavailable node.
+
+When *no* backing node answers, the route fails rather than returning an empty
+partial list: it surfaces the highest-severity per-node failure, so a
+node-unavailability outage returns ``503`` while a purely internal failure -
+for example every backing node reporting shutdown - returns ``500``. This route
+could not return ``500`` before; clients that previously saw ``503`` for a
+manager shutdown now see ``500``.
+
 Capability Description (OpenAPI Docs)
 --------------------------------------
 

src/ros2_medkit_gateway/CMakeLists.txt

@@ -593,6 +593,10 @@ if(BUILD_TESTING)
   ament_add_gtest(test_error_info test/test_error_info.cpp)
   target_link_libraries(test_error_info gateway_ros2)
 
+  # Parameter error classification unit tests (pure C++, no rclcpp)
+  ament_add_gtest(test_parameter_error_classification test/test_parameter_error_classification.cpp)
+  target_link_libraries(test_parameter_error_classification gateway_ros2)
+
   # Parameter error precedence tests (multi-node GET aggregation, live GatewayNode)
   ament_add_gtest(test_parameter_error_precedence test/test_parameter_error_precedence.cpp)
   target_link_libraries(test_parameter_error_precedence gateway_ros2)

src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/parameter_error_classification.hpp

@@ -0,0 +1,38 @@
+// Copyright 2026 bburda
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#pragma once
+
+#include <string>
+
+#include "ros2_medkit_gateway/core/configuration/parameter_types.hpp"
+
+namespace ros2_medkit_gateway {
+namespace handlers {
+
+/// HTTP status + SOVD error code for a failed parameter operation.
+struct ParameterErrorClassification {
+  int status_code = 500;
+  std::string error_code;
+};
+
+/// Map a failed `ParameterResult` to an HTTP status + SOVD error code from its
+/// structured `error_code`. Every failure path on the shipped
+/// `Ros2ParameterTransport` sets a structured code, so a failure that still
+/// carries `ParameterErrorCode::NONE` is a gateway-side defect and maps to 500
+/// internal-error - it is never guessed from the free-form message text.
+ParameterErrorClassification classify_parameter_error(const ParameterResult & result);
+
+}  // namespace handlers
+}  // namespace ros2_medkit_gateway

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/config.hpp

@@ -87,8 +87,11 @@ inline constexpr std::string_view dto_name<ConfigurationMetaData> = "Configurati
 //   parameters        - full parameter details including value/type/read_only
 //                       (free-form: raw ConfigurationManager output + x-medkit)
 //   source_ids        - namespace/FQN strings used for node lookup
-//   queried_nodes     - list of node FQNs successfully queried
-//   partial            - true when a fan-out peer request failed
+//   queried_nodes      - list of node FQNs successfully queried
+//   unavailable_nodes  - backing node FQNs that were down/unresponsive (503-class)
+//   failed_nodes       - backing node FQNs that failed for a non-availability reason
+//   partial            - true when a fan-out peer request failed OR a local backing
+//                        node did not answer (see unavailable_nodes/failed_nodes)
 //   failed_peers       - list of peer addresses that returned errors
 //   peer_dropped_items - per-peer items dropped due to malformed JSON
 //                        (observability for invisible drift)
@@ -101,6 +104,8 @@ struct ConfigListXMedkit {
   std::optional<nlohmann::json> parameters;  // free-form: array of raw param JSON
   std::optional<std::vector<std::string>> source_ids;
   std::optional<std::vector<std::string>> queried_nodes;
+  std::optional<std::vector<std::string>> unavailable_nodes;
+  std::optional<std::vector<std::string>> failed_nodes;
   std::optional<bool> partial;
   std::optional<std::vector<std::string>> failed_peers;
   std::optional<std::vector<DroppedItem>> peer_dropped_items;
@@ -112,7 +117,9 @@ inline constexpr auto dto_fields<ConfigListXMedkit> = std::make_tuple(
     field("aggregation_level", &ConfigListXMedkit::aggregation_level),
     field("is_aggregated", &ConfigListXMedkit::is_aggregated), field("parameters", &ConfigListXMedkit::parameters),
     field("source_ids", &ConfigListXMedkit::source_ids), field("queried_nodes", &ConfigListXMedkit::queried_nodes),
-    field("partial", &ConfigListXMedkit::partial), field("failed_peers", &ConfigListXMedkit::failed_peers),
+    field("unavailable_nodes", &ConfigListXMedkit::unavailable_nodes),
+    field("failed_nodes", &ConfigListXMedkit::failed_nodes), field("partial", &ConfigListXMedkit::partial),
+    field("failed_peers", &ConfigListXMedkit::failed_peers),
     field("peer_dropped_items", &ConfigListXMedkit::peer_dropped_items));
 
 template <>
@@ -248,7 +255,8 @@ inline constexpr std::string_view dto_name<ConfigurationDeleteMultiStatus> = "Co
 // Same wire shape as the legacy `Collection<ConfigurationMetaData>` named
 // `ConfigurationList`, but the `x-medkit` payload carries the rich
 // `ConfigListXMedkit` fields (entity_id, source, parameters, aggregation_level,
-// is_aggregated, source_ids, queried_nodes, partial, failed_peers,
+// is_aggregated, source_ids, queried_nodes, unavailable_nodes, failed_nodes,
+// partial, failed_peers,
 // peer_dropped_items) instead of the generic `XMedkitCollection`. The schema
 // name is kept identical to the legacy `ConfigurationList` $ref so existing
 // OpenAPI clients are not affected; the difference is purely server-side

src/ros2_medkit_gateway/src/core/http/parameter_error_classification.cpp

@@ -0,0 +1,76 @@
+// Copyright 2026 bburda
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "ros2_medkit_gateway/core/http/parameter_error_classification.hpp"
+
+#include "ros2_medkit_gateway/core/http/error_codes.hpp"
+
+namespace ros2_medkit_gateway {
+namespace handlers {
+
+namespace {
+
+/// Map a structured `ParameterErrorCode` to an HTTP status + SOVD error code.
+ParameterErrorClassification classify_error_code(ParameterErrorCode error_code) {
+  ParameterErrorClassification result;
+
+  switch (error_code) {
+    case ParameterErrorCode::NOT_FOUND:
+    case ParameterErrorCode::NO_DEFAULTS_CACHED:
+      result.status_code = 404;
+      result.error_code = ERR_RESOURCE_NOT_FOUND;
+      break;
+    case ParameterErrorCode::READ_ONLY:
+      result.status_code = 403;
+      result.error_code = ERR_X_MEDKIT_ROS2_PARAMETER_READ_ONLY;
+      break;
+    case ParameterErrorCode::SERVICE_UNAVAILABLE:
+    case ParameterErrorCode::TIMEOUT:
+      result.status_code = 503;
+      result.error_code = ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE;
+      break;
+    case ParameterErrorCode::TYPE_MISMATCH:
+    case ParameterErrorCode::INVALID_VALUE:
+      result.status_code = 400;
+      result.error_code = ERR_INVALID_PARAMETER;
+      break;
+    case ParameterErrorCode::NONE:
+      // A failed ParameterResult that still carries NONE cannot occur on the
+      // shipped transport (every failure path sets a structured code), so a
+      // NONE failure is a gateway-side defect: surface it as 500 internal-error.
+      // The removed legacy string-matching fallback was unreachable, and would
+      // have misrouted messages it did not recognise (e.g. "did not respond",
+      // "not currently set") to 400 invalid-request had it ever run.
+      result.status_code = 500;
+      result.error_code = ERR_INTERNAL_ERROR;
+      break;
+    case ParameterErrorCode::SHUT_DOWN:
+    case ParameterErrorCode::INTERNAL_ERROR:
+    default:
+      result.status_code = 500;
+      result.error_code = ERR_INTERNAL_ERROR;
+      break;
+  }
+
+  return result;
+}
+
+}  // namespace
+
+ParameterErrorClassification classify_parameter_error(const ParameterResult & result) {
+  return classify_error_code(result.error_code);
+}
+
+}  // namespace handlers
+}  // namespace ros2_medkit_gateway

src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp

@@ -27,6 +27,7 @@
 #include "ros2_medkit_gateway/core/http/error_codes.hpp"
 #include "ros2_medkit_gateway/core/http/fan_out_helpers.hpp"
 #include "ros2_medkit_gateway/core/http/http_utils.hpp"
+#include "ros2_medkit_gateway/core/http/parameter_error_classification.hpp"
 #include "ros2_medkit_gateway/dto/json_reader.hpp"
 #include "ros2_medkit_gateway/dto/json_writer.hpp"
 #include "ros2_medkit_gateway/gateway_node.hpp"
@@ -114,90 +115,6 @@ const NodeConfigInfo * find_node_for_app(const std::vector<NodeConfigInfo> & nod
   return nullptr;
 }
 
-/// HTTP status + SOVD error code for a failed parameter operation.
-struct ParameterErrorClassification {
-  int status_code = 500;
-  std::string error_code;
-};
-
-/// Map a structured `ParameterErrorCode` to an HTTP status + SOVD error code.
-/// Identical mapping to the legacy `classify_error_code` helper.
-ParameterErrorClassification classify_error_code(ParameterErrorCode error_code) {
-  ParameterErrorClassification result;
-
-  switch (error_code) {
-    case ParameterErrorCode::NOT_FOUND:
-    case ParameterErrorCode::NO_DEFAULTS_CACHED:
-      result.status_code = 404;
-      result.error_code = ERR_RESOURCE_NOT_FOUND;
-      break;
-    case ParameterErrorCode::READ_ONLY:
-      result.status_code = 403;
-      result.error_code = ERR_X_MEDKIT_ROS2_PARAMETER_READ_ONLY;
-      break;
-    case ParameterErrorCode::SERVICE_UNAVAILABLE:
-    case ParameterErrorCode::TIMEOUT:
-      result.status_code = 503;
-      result.error_code = ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE;
-      break;
-    case ParameterErrorCode::TYPE_MISMATCH:
-    case ParameterErrorCode::INVALID_VALUE:
-      result.status_code = 400;
-      result.error_code = ERR_INVALID_PARAMETER;
-      break;
-    case ParameterErrorCode::NONE:
-      // NONE means "no error" - caller (classify_parameter_error) pre-filters
-      // this case, so reaching here indicates a programming error.
-      result.status_code = 500;
-      result.error_code = ERR_INTERNAL_ERROR;
-      break;
-    case ParameterErrorCode::SHUT_DOWN:
-    case ParameterErrorCode::INTERNAL_ERROR:
-    default:
-      result.status_code = 500;
-      result.error_code = ERR_INTERNAL_ERROR;
-      break;
-  }
-
-  return result;
-}
-
-/// Classify a ParameterResult into HTTP status + error code. Prefers the
-/// structured `error_code` field and falls back to string-matching the legacy
-/// error messages for backward compatibility with older transports.
-ParameterErrorClassification classify_parameter_error(const ParameterResult & result) {
-  // Prefer structured error code
-  if (result.error_code != ParameterErrorCode::NONE) {
-    return classify_error_code(result.error_code);
-  }
-
-  // Fallback to string parsing for backward compatibility
-  ParameterErrorClassification classification;
-  const auto & error_message = result.error_message;
-
-  if (error_message.find("not found") != std::string::npos ||
-      error_message.find("Parameter not found") != std::string::npos) {
-    classification.status_code = 404;
-    classification.error_code = ERR_RESOURCE_NOT_FOUND;
-  } else if (error_message.find("read-only") != std::string::npos ||
-             error_message.find("read only") != std::string::npos ||
-             error_message.find("is read_only") != std::string::npos) {
-    classification.status_code = 403;
-    classification.error_code = ERR_X_MEDKIT_ROS2_PARAMETER_READ_ONLY;
-  } else if (error_message.find("not available") != std::string::npos ||
-             error_message.find("service not available") != std::string::npos ||
-             error_message.find("timed out") != std::string::npos ||
-             error_message.find("timeout") != std::string::npos) {
-    classification.status_code = 503;
-    classification.error_code = ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE;
-  } else {
-    classification.status_code = 400;
-    classification.error_code = ERR_INVALID_REQUEST;
-  }
-
-  return classification;
-}
-
 /// Build a typed `ErrorInfo` for a failed parameter operation. Mirrors the
 /// legacy `send_parameter_error` helper's wire shape (params: details +
 /// entity_id + id, message: "Failed to <op> parameter").
@@ -409,8 +326,9 @@ ConfigHandlers::list_configurations(const http::TypedRequest & req) {
     const auto node_results = query_nodes_in_parallel(config_mgr, agg_configs);
 
     bool any_success = false;
-    std::string first_error;
-    std::string first_error_node;  // Track which node failed for better diagnostics
+    ParameterErrorAccumulator errors;            // worst failure to surface if no node answers
+    std::vector<std::string> unavailable_nodes;  // 503-class: node down/unresponsive
+    std::vector<std::string> failed_nodes;       // other per-node failures
 
     for (const auto & qr : node_results) {
       const auto & node_info = qr.node_info;
@@ -435,16 +353,35 @@ ConfigHandlers::list_configurations(const http::TypedRequest & req) {
             all_parameters.push_back(std::move(param_with_source));
           }
         }
-      } else if (first_error.empty()) {
-        first_error = result.error_message;
-        first_error_node = node_info.node_fqn;
+      } else {
+        errors.add(result);
+        // A per-node failure never discards the nodes that answered. A 503-class
+        // failure (node down/unresponsive) is an availability gap named in
+        // `unavailable_nodes`; any other failure is named in `failed_nodes`.
+        // Both keep the answering nodes' parameters and flag the list partial.
+        if (classify_parameter_error(result).status_code == 503) {
+          unavailable_nodes.push_back(node_info.node_fqn);
+        } else {
+          failed_nodes.push_back(node_info.node_fqn);
+        }
       }
     }
 
     if (!any_success) {
+      // No backing node answered. Surface the worst failure order-independently
+      // (`severity_rank`: 503 outranks other 5xx), so a mixed down + internal
+      // outage returns 503 to match GET /{entity}/configurations/{param}; name
+      // every failed node so the outage is diagnosable.
+      const auto & cls = errors.classification();
+      json details{{"details", errors.worst().error_message}, {"entity_id", entity_id}};
+      if (!unavailable_nodes.empty()) {
+        details["unavailable_nodes"] = unavailable_nodes;
+      }
+      if (!failed_nodes.empty()) {
+        details["failed_nodes"] = failed_nodes;
+      }
       return tl::unexpected(
-          make_error(503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, "Failed to list parameters from any node",
-                     json{{"details", first_error}, {"entity_id", entity_id}, {"failed_node", first_error_node}}));
+          make_error(cls.status_code, cls.error_code, "Failed to list parameters from any node", std::move(details)));
     }
 
     xm.parameters = all_parameters;
@@ -454,6 +391,22 @@ ConfigHandlers::list_configurations(const http::TypedRequest & req) {
     if (!queried_nodes.empty()) {
       xm.queried_nodes = queried_nodes;
     }
+    // Some (but not all) backing nodes failed. Mirror the peer fan-out honesty
+    // convention (partial + failed_peers) for local nodes: keep the answering
+    // nodes' parameters, flag the response partial, and name what failed -
+    // 503-class nodes in `unavailable_nodes`, other failures in `failed_nodes` -
+    // instead of silently returning a shrunken 200. Keeps the list surface
+    // consistent with GET /{entity}/configurations/{param}, which returns the
+    // same 503 for a backing-node outage.
+    if (!unavailable_nodes.empty() || !failed_nodes.empty()) {
+      xm.partial = true;
+    }
+    if (!unavailable_nodes.empty()) {
+      xm.unavailable_nodes = std::move(unavailable_nodes);
+    }
+    if (!failed_nodes.empty()) {
+      xm.failed_nodes = std::move(failed_nodes);
+    }
   }
 
   // Typed fan-out for collection endpoints. Replaces the legacy raw-JSON