From e0811ca1ed307936d7863b34921774bc8128cab4 Mon Sep 17 00:00:00 2001 From: Robin Dumas Date: Mon, 18 Nov 2024 15:17:18 -0800 Subject: [PATCH 1/3] Improve error handling: list invalid parameters and dedupe error log messages --- .../src/ros1_foxglove_bridge_nodelet.cpp | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp b/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp index 3b7fe2a..4d45380 100644 --- a/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp +++ b/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -656,6 +657,7 @@ class FoxgloveBridge : public nodelet::Nodelet { bool success = true; std::vector params; + std::vector invalidParams; for (const auto& paramName : parameterNames) { if (!isWhitelisted(paramName, _paramWhitelistPatterns)) { if (allParametersRequested) { @@ -665,6 +667,9 @@ class FoxgloveBridge : public nodelet::Nodelet { success = false; } } + if (_invalidParams.find(paramName) != _invalidParams.end()) { + continue; + } try { XmlRpc::XmlRpcValue value; @@ -672,12 +677,15 @@ class FoxgloveBridge : public nodelet::Nodelet { params.push_back(fromRosParam(paramName, value)); } catch (const std::exception& ex) { ROS_ERROR("Invalid parameter '%s': %s", paramName.c_str(), ex.what()); + invalidParams.push_back(paramName); success = false; } catch (const XmlRpc::XmlRpcException& ex) { ROS_ERROR("Invalid parameter '%s': %s", paramName.c_str(), ex.getMessage().c_str()); + invalidParams.push_back(paramName); success = false; } catch (...) { ROS_ERROR("Invalid parameter '%s'", paramName.c_str()); + invalidParams.push_back(paramName); success = false; } } @@ -685,7 +693,24 @@ class FoxgloveBridge : public nodelet::Nodelet { _server->publishParameterValues(hdl, params, requestId); if (!success) { - throw std::runtime_error("Failed to retrieve one or multiple parameters"); + + for (std::string& param: invalidParams) { + _invalidParams.insert(param); + } + + if (!invalidParams.empty()) { + std::ostringstream errorMsg; + errorMsg << "Failed to retrieve the following parameters: "; + for (size_t i = 0; i < invalidParams.size(); i++) { + errorMsg << invalidParams[i]; + if (i < invalidParams.size() - 1) { + errorMsg << ", "; + } + } + throw std::runtime_error(errorMsg.str()); + } else { + throw std::runtime_error("Failed to retrieve one or multiple parameters"); + } } } @@ -922,6 +947,7 @@ class FoxgloveBridge : public nodelet::Nodelet { int _serviceRetrievalTimeoutMs = DEFAULT_SERVICE_TYPE_RETRIEVAL_TIMEOUT_MS; std::atomic _subscribeGraphUpdates = false; std::unique_ptr _fetchAssetQueue; + std::unordered_set _invalidParams; }; } // namespace foxglove_bridge From eaffb6f61938db59162a03b4788911a942df019f Mon Sep 17 00:00:00 2001 From: Robin Dumas Date: Mon, 18 Nov 2024 16:29:31 -0800 Subject: [PATCH 2/3] Fixed formatting --- ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp b/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp index 4d45380..28a1568 100644 --- a/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp +++ b/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp @@ -4,9 +4,9 @@ #include #include #include +#include #include #include -#include #include #include @@ -693,8 +693,7 @@ class FoxgloveBridge : public nodelet::Nodelet { _server->publishParameterValues(hdl, params, requestId); if (!success) { - - for (std::string& param: invalidParams) { + for (std::string& param : invalidParams) { _invalidParams.insert(param); } From c2d646e485d08e3ea3775c5aafd7a97d2feaf1ae Mon Sep 17 00:00:00 2001 From: Robin Dumas Date: Wed, 20 Nov 2024 15:15:41 -0800 Subject: [PATCH 3/3] Added upper bound on number of invalid params tracked --- .../src/ros1_foxglove_bridge_nodelet.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp b/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp index 28a1568..72bab6f 100644 --- a/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp +++ b/ros1_foxglove_bridge/src/ros1_foxglove_bridge_nodelet.cpp @@ -4,7 +4,6 @@ #include #include #include -#include #include #include @@ -49,6 +48,7 @@ constexpr uint32_t SUBSCRIPTION_QUEUE_LENGTH = 10; constexpr double MIN_UPDATE_PERIOD_MS = 100.0; constexpr uint32_t PUBLICATION_QUEUE_LENGTH = 10; constexpr int DEFAULT_SERVICE_TYPE_RETRIEVAL_TIMEOUT_MS = 250; +constexpr int MAX_INVALID_PARAMS_TRACKED = 1000; using ConnectionHandle = websocketpp::connection_hdl; using TopicAndDatatype = std::pair; @@ -694,19 +694,20 @@ class FoxgloveBridge : public nodelet::Nodelet { if (!success) { for (std::string& param : invalidParams) { - _invalidParams.insert(param); + if (_invalidParams.size() < MAX_INVALID_PARAMS_TRACKED) { + _invalidParams.insert(param); + } } if (!invalidParams.empty()) { - std::ostringstream errorMsg; - errorMsg << "Failed to retrieve the following parameters: "; + std::string errorMsg = "Failed to retrieve the following parameters: "; for (size_t i = 0; i < invalidParams.size(); i++) { - errorMsg << invalidParams[i]; + errorMsg += invalidParams[i]; if (i < invalidParams.size() - 1) { - errorMsg << ", "; + errorMsg += ", "; } } - throw std::runtime_error(errorMsg.str()); + throw std::runtime_error(errorMsg); } else { throw std::runtime_error("Failed to retrieve one or multiple parameters"); }