bmcweb: Set Retry Policy Valid Response Codes

Allows individual retry policies to specify what HTTP response codes
are considered valid.  Sets functions for the EventService and
Redfish Aggregation retry policies.  Those functions expect a
response code and return an error code based on what the response
code is.

This change is needed because EventService only considers 2XX codes
to be valid.  Any code outside of that range would trigger a retry
attempt.  Redfish Aggregation by design will need to return
errors outside of that range such as 404.  It should not retry to
send a message when it receives a 404 from a satellite BMC.

Right now 404 is the only error code that is handled differently
between the services.  Going forward, Redfish Aggregation will
likely want to allow other error codes as its functionality is
expanded.

Tested:
Used Redfish-Event-Listener with ssh port forwarding to create 3
subscriptions.  I then closed the ssh connection and sent a test
event.  Bmcweb made 3 retry attempts for each subscription.  At
that point the max retry amount (as defined by EventService) was
reached and bmcweb stop attempting to resend the messages.

There were no errors when the Redfish-Event-Listener was correctly
connected.  Test events resulted in messages being sent for each
subscription.

Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Ifdfaf638d28982ed18998f3ca05280a288e0020a

3 files changed, 72 insertions, 16 deletions

diff --git a/http/http_client.hpp b/http/http_client.hpp
index d35ddd446a..a28c1c4245 100644
--- a/http/http_client.hpp
+++ b/http/http_client.hpp
@@ -60,6 +60,21 @@ enum class ConnState
     retry
 };
 
+static inline boost::system::error_code
+    defaultRetryHandler(unsigned int respCode)
+{
+    // As a default, assume 200X is alright
+    BMCWEB_LOG_DEBUG << "Using default check for response code validity";
+    if ((respCode < 200) || (respCode >= 300))
+    {
+        return boost::system::errc::make_error_code(
+            boost::system::errc::result_out_of_range);
+    }
+
+    // Return 0 if the response code is valid
+    return boost::system::errc::make_error_code(boost::system::errc::success);
+};
+
 // We need to allow retry information to be set before a message has been sent
 // and a connection pool has been created
 struct RetryPolicyData
@@ -67,7 +82,8 @@ struct RetryPolicyData
     uint32_t maxRetryAttempts = 5;
     std::chrono::seconds retryIntervalSecs = std::chrono::seconds(0);
     std::string retryPolicyAction = "TerminateAfterRetries";
-    std::string name;
+    std::function<boost::system::error_code(unsigned int respCode)>
+        invalidResp = defaultRetryHandler;
 };
 
 struct PendingRequest
@@ -231,8 +247,9 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo>
             BMCWEB_LOG_DEBUG << "recvMessage() Header Response Code: "
                              << respCode;
 
-            // 2XX response is considered to be successful
-            if ((respCode < 200) || (respCode >= 300))
+            // Make sure the received response code is valid as defined by
+            // the associated retry policy
+            if (self->retryPolicy.invalidResp(respCode))
             {
                 // The listener failed to receive the Sent-Event
                 BMCWEB_LOG_ERROR << "recvMessage() Listener Failed to "
@@ -414,9 +431,7 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool>
 
         BMCWEB_LOG_DEBUG << "Setting properties for connection " << conn.host
                          << ":" << std::to_string(conn.port)
-                         << ", id: " << std::to_string(conn.connId)
-                         << ", retry policy is \"" << conn.retryPolicy.name
-                         << "\"";
+                         << ", id: " << std::to_string(conn.connId);
 
         // We can remove the request from the queue at this point
         requestQueue.pop_front();
@@ -427,9 +442,7 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool>
                                    const RetryPolicyData& retryPolicy)
     {
         BMCWEB_LOG_DEBUG << destIP << ":" << std::to_string(destPort)
-                         << ", id: " << std::to_string(conn.connId)
-                         << " using retry policy \"" << retryPolicy.name
-                         << "\"";
+                         << ", id: " << std::to_string(conn.connId);
 
         conn.retryPolicy = retryPolicy;
     }
@@ -678,7 +691,6 @@ class HttpClient
         {
             BMCWEB_LOG_DEBUG << "Creating retry policy \"" << retryPolicyName
                              << "\" with default values";
-            policy.first->second.name = retryPolicyName;
         }
 
         // Send the data using either the existing connection pool or the newly
@@ -687,9 +699,11 @@ class HttpClient
                                        policy.first->second, resHandler);
     }
 
-    void setRetryConfig(const uint32_t retryAttempts,
-                        const uint32_t retryTimeoutInterval,
-                        const std::string& retryPolicyName)
+    void setRetryConfig(
+        const uint32_t retryAttempts, const uint32_t retryTimeoutInterval,
+        const std::function<boost::system::error_code(unsigned int respCode)>&
+            invalidResp,
+        const std::string& retryPolicyName)
     {
         // We need to create the retry policy if one does not already exist for
         // the given retryPolicyName
@@ -698,7 +712,6 @@ class HttpClient
         {
             BMCWEB_LOG_DEBUG << "setRetryConfig(): Creating new retry policy \""
                              << retryPolicyName << "\"";
-            result.first->second.name = retryPolicyName;
         }
         else
         {
@@ -709,6 +722,7 @@ class HttpClient
         result.first->second.maxRetryAttempts = retryAttempts;
         result.first->second.retryIntervalSecs =
             std::chrono::seconds(retryTimeoutInterval);
+        result.first->second.invalidResp = invalidResp;
     }
 
     void setRetryPolicy(const std::string& retryPolicy,
@@ -721,7 +735,6 @@ class HttpClient
         {
             BMCWEB_LOG_DEBUG << "setRetryPolicy(): Creating new retry policy \""
                              << retryPolicyName << "\"";
-            result.first->second.name = retryPolicyName;
         }
         else
         {
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index 6747575c33..8c21fac5ab 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -537,7 +537,8 @@ class Subscription : public persistent_data::UserSubscription
                            const uint32_t retryTimeoutInterval)
     {
         crow::HttpClient::getInstance().setRetryConfig(
-            retryAttempts, retryTimeoutInterval, retryPolicyName);
+            retryAttempts, retryTimeoutInterval, retryRespHandler,
+            retryPolicyName);
     }
 
     void updateRetryPolicy()
@@ -559,6 +560,23 @@ class Subscription : public persistent_data::UserSubscription
     std::string uriProto;
     std::shared_ptr<crow::ServerSentEvents> sseConn = nullptr;
     std::string retryPolicyName = "SubscriptionEvent";
+
+    // Check used to indicate what response codes are valid as part of our retry
+    // policy.  2XX is considered acceptable
+    static boost::system::error_code retryRespHandler(unsigned int respCode)
+    {
+        BMCWEB_LOG_DEBUG
+            << "Checking response code validity for SubscriptionEvent";
+        if ((respCode < 200) || (respCode >= 300))
+        {
+            return boost::system::errc::make_error_code(
+                boost::system::errc::result_out_of_range);
+        }
+
+        // Return 0 if the response code is valid
+        return boost::system::errc::make_error_code(
+            boost::system::errc::success);
+    };
 };
 
 class EventServiceManager
diff --git a/redfish-core/include/redfish_aggregator.hpp b/redfish-core/include/redfish_aggregator.hpp
index 67ed929cb5..01e5aba45e 100644
--- a/redfish-core/include/redfish_aggregator.hpp
+++ b/redfish-core/include/redfish_aggregator.hpp
@@ -8,11 +8,36 @@ namespace redfish
 class RedfishAggregator
 {
   private:
+    const std::string retryPolicyName = "RedfishAggregation";
+    const uint32_t retryAttempts = 5;
+    const uint32_t retryTimeoutInterval = 0;
+
     RedfishAggregator()
     {
         getSatelliteConfigs(constructorCallback);
+
+        // Setup the retry policy to be used by Redfish Aggregation
+        crow::HttpClient::getInstance().setRetryConfig(
+            retryAttempts, retryTimeoutInterval, aggregationRetryHandler,
+            retryPolicyName);
     }
 
+    static inline boost::system::error_code
+        aggregationRetryHandler(unsigned int respCode)
+    {
+        // As a default, assume 200X is alright.
+        // We don't need to retry on a 404
+        if ((respCode < 200) || ((respCode >= 300) && (respCode != 404)))
+        {
+            return boost::system::errc::make_error_code(
+                boost::system::errc::result_out_of_range);
+        }
+
+        // Return 0 if the response code is valid
+        return boost::system::errc::make_error_code(
+            boost::system::errc::success);
+    };
+
     // Dummy callback used by the Constructor so that it can report the number
     // of satellite configs when the class is first created
     static void constructorCallback(