dashboard: get rid of oatpp by MariusBgm · Pull Request #419 · vectorgrp/sil-kit

MariusBgm · 2026-09-01T16:43:01Z

Drop oat++ from the SIL Kit Dashboard client

removed: ThirdParty/oatpp submodule, its CMake and license entries
added: minimal HTTP/1.1 client over the bundled standalone asio (dashboard/http/)
changed: dashboard JSON now emitted with the bundled rapidyaml
changed: wire format identical bar 3 cosmetics: space after :, no \/, UTF-8 over \uXXXX; unescapable control chars become U+FFFD
fixed: registry hung on shutdown when the dashboard accepted but never answered; requests now have connect/write/read deadlines and abort after a grace period
fixed: use-after-free, RegistryInstance destroyed the dashboard before the registry pointing at it
fixed: one unmappable event killed the worker thread, silently ending all dashboard reporting; failures are now logged and skipped per event and per flush
fixed: unhandled controller types threw instead of being skipped, reachable by 11 of 21 types
removed: bulk-update probe, which killed the worker on a negative result; also drops a startup round-trip and a ~7 s stall when unreachable
fixed: events pending at shutdown were discarded, now flushed
fixed: dashboard log lines had no Dashboard topic and escaped topic filtering
changed: SILKIT_BUILD_DASHBOARD=OFF with --dashboard-uri now reports the missing feature plainly
added: warning when the dashboard is on but CollectFromRemote is off, which silently yields no metrics or attributes
changed: SilKitEvent type erasure to std::variant, worker split out; net −348 lines
tests: new suites for HTTP client, parser, retry, JSON writer, DTO mapper, worker, shutdown; IRestClient mocked, batching had no coverage
tests: no sleeps or wall-clock assertions, latch handover, stable under CI load

Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>

refactor and cleanups Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>

Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>

VDanielEdwards

Heres a mixed set of comments made by Claude and me.

VDanielEdwards · 2026-09-03T08:45:58Z

+
+    asio::io_context ioContext{1};
+    std::optional<asio::ip::tcp::socket> socket;
+    asio::streambuf readBuffer;


Default max_size is SIZE_MAX, so ReadHead's async_read_until and ReadUntilClose's async_read never self-terminate — maxHeadSize/maxHttpBodySize are only checked once the data is already buffered. A server that streams headers without the blank line, or a response with neither Content-Length nor Transfer-Encoding, grows RSS for the whole 30s read window.

asio::streambuf readBuffer{maxHeadSize + maxHttpBodySize}; bounds both (async_read_until then completes with error::not_found). Content-Length and chunked framing are already bounded before reading.

VDanielEdwards · 2026-09-03T08:45:58Z

+        {
+            return asio::error::message_size;
+        }
+        const auto* data = asio::buffer_cast<const char*>(readBuffer.data());


asio::buffer_cast is deprecated in asio 1.30 — compiles only while ASIO_NO_DEPRECATED is unset. static_cast<const char*>(readBuffer.data().data()) is the current spelling. Also lines 277, 298, 368 and in FakeHttpServer.hpp.

VDanielEdwards · 2026-09-03T08:45:58Z

+
+TEST_F(Test_AsioHttpClient, Post_ReportsATransportErrorWhenNothingIsListening)
+{
+    // Port 1 is reserved and never has a listener. The connect deadline is out of reach, so a


1h connect deadline + port 1 assumes the host refuses rather than drops. Where loopback port 1 is filtered this hangs to the CTest timeout. Same in service/Test_DashboardShutdown.cpp:79.

VDanielEdwards · 2026-09-03T08:56:23Z

+    // Parse eagerly so that a malformed --dashboard-uri is reported when the instance is created,
+    // rather than later on the registry's thread. The result is discarded; DashboardRestClient
+    // parses it again once it is built.
+    (void)SilKit::Core::Uri::Parse(dashboardUri);


Consider making sure that the URI scheme is http. The Uri::Parse function accepts any scheme.

Suggested change

(void)SilKit::Core::Uri::Parse(dashboardUri);

const auto uri = SilKit::Core::Uri::Parse(dashboardUri);

if (uri.Scheme() != "http")

{

SilKit::SilKitError{"Dashboard URI must have the http scheme!"};

}

Alternatively the check could be done in DashboardRestClient.

VDanielEdwards · 2026-09-03T09:13:40Z

+    # Every dashboard test links the same set, so register them from one list.
+    set(SILKIT_DASHBOARD_TEST_SOURCES
+        Test_DashboardEventQueueWorker.cpp
+        Test_DashboardSilKitEventQueue.cpp
+        client/Test_DashboardSystemServiceClient.cpp
+        http/Test_AsioHttpClient.cpp
+        http/Test_HttpResponseParser.cpp
+        http/Test_RetryingHttpClient.cpp
+        json/Test_DashboardJsonWriter.cpp
+        service/Test_DashboardDtoMapper.cpp
+        service/Test_DashboardRestClient.cpp
+        service/Test_DashboardShutdown.cpp
    )

-    add_silkit_test_to_executable(SilKitDashboardTests
-        SOURCES client/Test_DashboardSystemServiceClient.cpp 
-        LIBS
-        S_SilKitImpl
-        O_SilKit_Dashboard
-        I_SilKit
-        oatpp
-    )
+    target_sources(SilKitDashboardTests PRIVATE http/FakeHttpServer.hpp Mocks/MockRestClient.hpp)
+
+    foreach(testSource IN LISTS SILKIT_DASHBOARD_TEST_SOURCES)
+        add_silkit_test_to_executable(SilKitDashboardTests
+            SOURCES ${testSource}
+            LIBS
+            S_SilKitImpl
+            O_SilKit_Dashboard
+            I_SilKit
+        )
+    endforeach()


Using target_sources is only valid when SILKIT_BUILD_TESTS=ON. Otherwise the target does not exist. All sources can be combined into a single call to add_silkit_test_to_executable. This does the check internally.

Suggested change

# Every dashboard test links the same set, so register them from one list.

set(SILKIT_DASHBOARD_TEST_SOURCES

Test_DashboardEventQueueWorker.cpp

Test_DashboardSilKitEventQueue.cpp

client/Test_DashboardSystemServiceClient.cpp

http/Test_AsioHttpClient.cpp

http/Test_HttpResponseParser.cpp

http/Test_RetryingHttpClient.cpp

json/Test_DashboardJsonWriter.cpp

service/Test_DashboardDtoMapper.cpp

service/Test_DashboardRestClient.cpp

service/Test_DashboardShutdown.cpp

)

add_silkit_test_to_executable(SilKitDashboardTests

SOURCES client/Test_DashboardSystemServiceClient.cpp

LIBS

S_SilKitImpl

O_SilKit_Dashboard

I_SilKit

oatpp

)

target_sources(SilKitDashboardTests PRIVATE http/FakeHttpServer.hpp Mocks/MockRestClient.hpp)

foreach(testSource IN LISTS SILKIT_DASHBOARD_TEST_SOURCES)

add_silkit_test_to_executable(SilKitDashboardTests

SOURCES ${testSource}

LIBS

S_SilKitImpl

O_SilKit_Dashboard

I_SilKit

)

endforeach()

add_silkit_test_to_executable(SilKitDashboardTests

SOURCES

http/FakeHttpServer.hpp

Mocks/MockRestClient.hpp

Test_DashboardEventQueueWorker.cpp

Test_DashboardSilKitEventQueue.cpp

client/Test_DashboardSystemServiceClient.cpp

http/Test_AsioHttpClient.cpp

http/Test_HttpResponseParser.cpp

http/Test_RetryingHttpClient.cpp

json/Test_DashboardJsonWriter.cpp

service/Test_DashboardDtoMapper.cpp

service/Test_DashboardRestClient.cpp

service/Test_DashboardShutdown.cpp

LIBS

S_SilKitImpl

O_SilKit_Dashboard

I_SilKit

)

VDanielEdwards · 2026-09-03T09:39:49Z

+        char buffer[64];
+        const int length = std::snprintf(buffer, sizeof buffer, "%.16g", value);
+        node << ryml::csubstr{buffer, static_cast<size_t>(length < 0 ? 0 : length)};


Since snprintf(..., "%.16g", ...) can emit nan or inf (for which JSON has no represantation in any case) we should handle this case here. I'd just emit 0. We could also add a log message in this cae, but then we'd have to keep a logger pointer around.

Suggested change

char buffer[64];

const int length = std::snprintf(buffer, sizeof buffer, "%.16g", value);

node << ryml::csubstr{buffer, static_cast<size_t>(length < 0 ? 0 : length)};

if (std::isfinite(value))

{

char buffer[64];

const int length = std::snprintf(buffer, sizeof buffer, "%.16g", value);

node << ryml::csubstr{buffer, static_cast<size_t>(length < 0 ? 0 : length)};

}

else

{

node << 0;

}

VDanielEdwards · 2026-09-03T09:57:57Z

+ *
+ *  Implementations must never throw out of Post(): every failure is reported as
+ *  HttpResult::transportError. Post() is expected to be called from a single thread; Abort() may be
+ *  called concurrently from another.


Can Reset() also only be called from the same, singular thread as Post(...)?

VDanielEdwards · 2026-09-03T10:17:54Z

+// asio types are kept out of this header so that the 22k-line asio headers are confined to the
+// single translation unit that implements the client.
+namespace asio {
+class io_context;


Is this forward declare neccessary? AFAICT its not used in this header, and the implementation includes the asio headers themselves.

dashboard: get rid of oatpp

8701f38

Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>

MariusBgm force-pushed the dev/get_rid_of_oatpp branch from 713ff9a to 8701f38 Compare September 1, 2026 16:47

MariusBgm added 4 commits September 1, 2026 19:53

fixup! dashboard: get rid of oatpp

94cdb19

refactor and cleanups Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>

test fixes

db13626

Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>

tested against dashboard v1.2.2. fixes

6ee96cf

Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>

scrub hungarian notation

4188516

Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>

MariusBgm added the needs reviewer This issue is looking for a reviewer. label Sep 2, 2026

MariusBgm marked this pull request as ready for review September 2, 2026 12:50

VDanielEdwards requested changes Sep 3, 2026

View reviewed changes

Navigation Menu

Uh oh!

dashboard: get rid of oatpp - #419

dashboard: get rid of oatpp#419
MariusBgm wants to merge 5 commits into
mainfrom
dev/get_rid_of_oatpp

MariusBgm commented Sep 1, 2026 •

edited

Loading

Uh oh!

VDanielEdwards left a comment

Uh oh!

VDanielEdwards Sep 3, 2026

Uh oh!

VDanielEdwards Sep 3, 2026

Uh oh!

VDanielEdwards Sep 3, 2026

Uh oh!

VDanielEdwards Sep 3, 2026

Uh oh!

VDanielEdwards Sep 3, 2026

Uh oh!

VDanielEdwards Sep 3, 2026

Uh oh!

VDanielEdwards Sep 3, 2026

Uh oh!

VDanielEdwards Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    (void)SilKit::Core::Uri::Parse(dashboardUri);
+    const auto uri = SilKit::Core::Uri::Parse(dashboardUri);
+    if (uri.Scheme() != "http")
+    {
+        SilKit::SilKitError{"Dashboard URI must have the http scheme!"};
+    }

Uh oh!

Conversation

MariusBgm commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Drop oat++ from the SIL Kit Dashboard client

Uh oh!

VDanielEdwards left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MariusBgm commented Sep 1, 2026 •

edited

Loading