fix: catch exception if en_US.utf8-locale missing when parsing datetime headers (#251)

The event delivery subsystem inspects the HTTP headers on the event
endpoints whenever it posts a batch of events.

It parses out the date header, storing it for later use. When emitting
debug events, it is used to ensure that events stop emitting even if the
host's time is off - as a form of time synchronization.

The parsing code is quite ugly, and also wrong - it assumed that
`en_US.utf-8` would always be available.

This commit hoists the locale loading routine out of the individual
flush-workers and into the worker pool. If it can't load the locale, it
emits a single `warn` log at startup explaining the effect.

Author: cwaldren-ld

libs/internal/include/launchdarkly/events/parse_date_header.hpp

@@ -8,14 +8,17 @@
 namespace launchdarkly::events {
 
 template <typename Clock>
+
 static std::optional<typename Clock::time_point> ParseDateHeader(
-    std::string const& datetime) {
+    std::string const& datetime,
+    std::locale const& locale) {
     // The following comments may not be entirely accurate.
     // TODO: There must be a better way.
 
     std::tm gmt_tm = {};
+
     std::istringstream string_stream(datetime);
-    string_stream.imbue(std::locale("en_US.utf-8"));
+    string_stream.imbue(locale);
     string_stream >> std::get_time(&gmt_tm, "%a, %d %b %Y %H:%M:%S GMT");
     if (string_stream.fail()) {
         return std::nullopt;

libs/internal/include/launchdarkly/events/request_worker.hpp

@@ -24,7 +24,7 @@ enum class State {
     PermanentlyFailed = 4,
 };
 
-std::ostream& operator<<(std::ostream& out, State const& s);
+std::ostream& operator<<(std::ostream& out, State const& state);
 
 enum class Action {
     /* No action necessary. */
@@ -39,7 +39,7 @@ enum class Action {
     NotifyPermanentFailure = 4,
 };
 
-std::ostream& operator<<(std::ostream& out, Action const& s);
+std::ostream& operator<<(std::ostream& out, Action const& state);
 
 /**
  * Computes the next (state, action) pair from an existing state and an HTTP
@@ -99,12 +99,13 @@ class RequestWorker {
     RequestWorker(boost::asio::any_io_executor io,
                   std::chrono::milliseconds retry_after,
                   std::size_t id,
+                  std::optional<std::locale> date_header_locale,
                   Logger& logger);
 
     /**
      * Returns true if the worker is available for delivery.
      */
-    bool Available() const;
+    [[nodiscard]] bool Available() const;
 
     /**
      * Passes an EventBatch to the worker for delivery. The delivery may be
@@ -135,10 +136,10 @@ class RequestWorker {
             << batch_->Target() << " with payload: "
             << batch_->Request().Body().value_or("(no body)");
 
-        requester_.Request(
-            batch_->Request(), [this, handler](network::HttpResult result) {
-                OnDeliveryAttempt(std::move(result), std::move(handler));
-            });
+        requester_.Request(batch_->Request(),
+                           [this, handler](network::HttpResult const& result) {
+                               OnDeliveryAttempt(result, std::move(handler));
+                           });
         return result.get();
     }
 
@@ -163,9 +164,14 @@ class RequestWorker {
     /* Tag used in logs. */
     std::string tag_;
 
+    /* The en_US locale is used to parse the Date header from HTTP responses.
+     * On some platforms, this may not be available hence the optional. */
+    std::optional<std::locale> date_header_locale_;
+
     Logger& logger_;
 
-    void OnDeliveryAttempt(network::HttpResult request, ResultCallback cb);
+    void OnDeliveryAttempt(network::HttpResult const& request,
+                           ResultCallback cb);
 };
 
 }  // namespace launchdarkly::events

libs/internal/include/launchdarkly/events/worker_pool.hpp

@@ -24,8 +24,6 @@ namespace launchdarkly::events {
  */
 class WorkerPool {
    public:
-    using ServerTimeCallback =
-        std::function<void(std::chrono::system_clock::time_point)>;
     /**
      * Constructs a new WorkerPool.
      * @param io The executor used for all workers.

libs/internal/src/events/request_worker.cpp

@@ -6,13 +6,15 @@ namespace launchdarkly::events {
 RequestWorker::RequestWorker(boost::asio::any_io_executor io,
                              std::chrono::milliseconds retry_after,
                              std::size_t id,
+                             std::optional<std::locale> date_header_locale,
                              Logger& logger)
-    : timer_(io),
+    : timer_(std::move(io)),
       retry_delay_(retry_after),
       state_(State::Idle),
       requester_(timer_.get_executor()),
       batch_(std::nullopt),
       tag_("flush-worker[" + std::to_string(id) + "]: "),
+      date_header_locale_(std::move(date_header_locale)),
       logger_(logger) {}
 
 bool RequestWorker::Available() const {
@@ -47,7 +49,7 @@ static bool IsSuccess(network::HttpResult const& result) {
                http::status_class::successful;
 }
 
-void RequestWorker::OnDeliveryAttempt(network::HttpResult result,
+void RequestWorker::OnDeliveryAttempt(network::HttpResult const& result,
                                       ResultCallback callback) {
     auto [next_state, action] = NextState(state_, result);
 
@@ -81,11 +83,15 @@ void RequestWorker::OnDeliveryAttempt(network::HttpResult result,
             batch_.reset();
             break;
         case Action::ParseDateAndReset: {
+            if (!date_header_locale_) {
+                batch_.reset();
+                break;
+            }
             auto headers = result.Headers();
             if (auto date = headers.find("Date"); date != headers.end()) {
                 if (auto server_time =
                         ParseDateHeader<std::chrono::system_clock>(
-                            date->second)) {
+                            date->second, *date_header_locale_)) {
                     callback(batch_->Count(), *server_time);
                 }
             }

libs/internal/src/events/worker_pool.cpp

@@ -5,14 +5,37 @@
 
 namespace launchdarkly::events {
 
+std::optional<std::locale> GetLocale(std::string const& locale,
+                                     std::string const& tag,
+                                     Logger& logger) {
+    try {
+        return std::locale(locale);
+    } catch (std::runtime_error) {
+        LD_LOG(logger, LogLevel::kWarn)
+            << tag << " couldn't load " << locale
+            << " locale. If debug events are enabled, they may be emitted for "
+               "longer than expected";
+        return std::nullopt;
+    }
+}
+
 WorkerPool::WorkerPool(boost::asio::any_io_executor io,
                        std::size_t pool_size,
                        std::chrono::milliseconds delivery_retry_delay,
                        Logger& logger)
     : io_(io), workers_() {
+    // The en_US.utf-8 locale is used whenever a date is parsed from the HTTP
+    // headers returned by the event-delivery endpoints. If the locale is
+    // unavailable, then the workers will skip the parsing step.
+    //
+    // This may result in debug events being emitted for longer than expected
+    // if the host's time is way out of sync.
+    std::optional<std::locale> date_header_locale =
+        GetLocale("en_US.utf-8", "event-processor", logger);
+
     for (std::size_t i = 0; i < pool_size; i++) {
         workers_.emplace_back(std::make_unique<RequestWorker>(
-            io_, delivery_retry_delay, i, logger));
+            io_, delivery_retry_delay, i, date_header_locale, logger));
     }
 }
 

libs/internal/tests/event_processor_test.cpp

@@ -59,9 +59,15 @@ TEST(WorkerPool, PoolReturnsNullptrWhenNoWorkerAvaialable) {
     ioc_thread.join();
 }
 
+class EventProcessorTests : public ::testing::Test {
+   public:
+    EventProcessorTests() : locale("en_US.utf-8") {}
+    std::locale locale;
+};
+
 // This test is a temporary test that exists only to ensure the event processor
 // compiles; it should be replaced by more robust tests (and contract tests.)
-TEST(EventProcessorTests, ProcessorCompiles) {
+TEST_F(EventProcessorTests, ProcessorCompiles) {
     using namespace launchdarkly;
 
     Logger logger{
@@ -95,33 +101,34 @@ TEST(EventProcessorTests, ProcessorCompiles) {
     ioc_thread.join();
 }
 
-TEST(EventProcessorTests, ParseValidDateHeader) {
+TEST_F(EventProcessorTests, ParseValidDateHeader) {
     using namespace launchdarkly;
 
     using Clock = std::chrono::system_clock;
-    auto date = events::ParseDateHeader<Clock>("Wed, 21 Oct 2015 07:28:00 GMT");
+    auto date =
+        events::ParseDateHeader<Clock>("Wed, 21 Oct 2015 07:28:00 GMT", locale);
 
     ASSERT_TRUE(date);
 
     ASSERT_EQ(date->time_since_epoch(),
               std::chrono::microseconds(1445412480000000));
 }
 
-TEST(EventProcessorTests, ParseInvalidDateHeader) {
+TEST_F(EventProcessorTests, ParseInvalidDateHeader) {
     using namespace launchdarkly;
 
     auto not_a_date = events::ParseDateHeader<std::chrono::system_clock>(
-        "this is definitely not a date");
+        "this is definitely not a date", locale);
 
     ASSERT_FALSE(not_a_date);
 
     auto not_gmt = events::ParseDateHeader<std::chrono::system_clock>(
-        "Wed, 21 Oct 2015 07:28:00 PST");
+        "Wed, 21 Oct 2015 07:28:00 PST", locale);
 
     ASSERT_FALSE(not_gmt);
 
     auto missing_year = events::ParseDateHeader<std::chrono::system_clock>(
-        "Wed, 21 Oct 07:28:00 GMT");
+        "Wed, 21 Oct 07:28:00 GMT", locale);
 
     ASSERT_FALSE(missing_year);
 }