From 28be546e2fe56de608fa4404907e3b78ade13fc8 Mon Sep 17 00:00:00 2001 From: Minggang Wang Date: Wed, 14 Jan 2026 14:32:52 +0800 Subject: [PATCH 1/6] Fix subscription content-filtering flakiness --- src/rcl_subscription_bindings.cpp | 79 ++++++++++++++++++++++++------- 1 file changed, 61 insertions(+), 18 deletions(-) diff --git a/src/rcl_subscription_bindings.cpp b/src/rcl_subscription_bindings.cpp index b880245f..d1bf59d2 100644 --- a/src/rcl_subscription_bindings.cpp +++ b/src/rcl_subscription_bindings.cpp @@ -99,7 +99,7 @@ Napi::Value CreateSubscription(const Napi::CallbackInfo& info) { for (int i = 0; i < argc; i++) { std::string arg = jsArgv.Get(i).As().Utf8Value(); int len = arg.length() + 1; - argv[i] = reinterpret_cast(malloc(len * sizeof(char*))); + argv[i] = reinterpret_cast(malloc(len * sizeof(char))); snprintf(argv[i], len, "%s", arg.c_str()); } } @@ -235,7 +235,7 @@ Napi::Value SetContentFilter(const Napi::CallbackInfo& info) { for (int i = 0; i < argc; i++) { std::string arg = jsArgv.Get(i).As().Utf8Value(); int len = arg.length() + 1; - argv[i] = reinterpret_cast(malloc(len * sizeof(char*))); + argv[i] = reinterpret_cast(malloc(len * sizeof(char))); snprintf(argv[i], len, "%s", arg.c_str()); } } @@ -245,15 +245,23 @@ Napi::Value SetContentFilter(const Napi::CallbackInfo& info) { rcl_subscription_content_filter_options_t options = rcl_get_zero_initialized_subscription_content_filter_options(); - THROW_ERROR_IF_NOT_EQUAL( - RCL_RET_OK, - rcl_subscription_content_filter_options_set( - subscription, expression.c_str(), argc, (const char**)argv, &options), - rcl_get_error_string().str); + rcl_ret_t ret = rcl_subscription_content_filter_options_set( + subscription, expression.c_str(), argc, (const char**)argv, &options); - THROW_ERROR_IF_NOT_EQUAL( - RCL_RET_OK, rcl_subscription_set_content_filter(subscription, &options), - rcl_get_error_string().str); + if (ret != RCL_RET_OK) { + if (argc) { + for (int i = 0; i < argc; i++) { + free(argv[i]); + } + free(argv); + } + rcl_reset_error(); + Napi::Error::New(env, rcl_get_error_string().str) + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + + ret = rcl_subscription_set_content_filter(subscription, &options); if (argc) { for (int i = 0; i < argc; i++) { @@ -262,6 +270,23 @@ Napi::Value SetContentFilter(const Napi::CallbackInfo& info) { free(argv); } + rcl_ret_t fini_ret = + rcl_subscription_content_filter_options_fini(subscription, &options); + + if (ret != RCL_RET_OK) { + rcl_reset_error(); + Napi::Error::New(env, rcl_get_error_string().str) + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + + if (fini_ret != RCL_RET_OK) { + rcl_reset_error(); + Napi::Error::New(env, rcl_get_error_string().str) + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + return Napi::Boolean::New(env, true); } @@ -277,15 +302,33 @@ Napi::Value ClearContentFilter(const Napi::CallbackInfo& info) { rcl_subscription_content_filter_options_t options = rcl_get_zero_initialized_subscription_content_filter_options(); - THROW_ERROR_IF_NOT_EQUAL( - RCL_RET_OK, - rcl_subscription_content_filter_options_init( - subscription, "", 0, (const char**)nullptr, &options), - rcl_get_error_string().str); + rcl_ret_t ret = rcl_subscription_content_filter_options_init( + subscription, "", 0, (const char**)nullptr, &options); - THROW_ERROR_IF_NOT_EQUAL( - RCL_RET_OK, rcl_subscription_set_content_filter(subscription, &options), - rcl_get_error_string().str); + if (ret != RCL_RET_OK) { + rcl_reset_error(); + Napi::Error::New(env, rcl_get_error_string().str) + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + + ret = rcl_subscription_set_content_filter(subscription, &options); + rcl_ret_t fini_ret = + rcl_subscription_content_filter_options_fini(subscription, &options); + + if (ret != RCL_RET_OK) { + rcl_reset_error(); + Napi::Error::New(env, rcl_get_error_string().str) + .ThrowAsJavaScriptException(); + return env.Undefined(); + } + + if (fini_ret != RCL_RET_OK) { + rcl_reset_error(); + Napi::Error::New(env, rcl_get_error_string().str) + .ThrowAsJavaScriptException(); + return env.Undefined(); + } return Napi::Boolean::New(env, true); } From ca058bf9ba11652194be10bcb14a4affc6d4b647 Mon Sep 17 00:00:00 2001 From: Minggang Wang Date: Wed, 14 Jan 2026 14:51:22 +0800 Subject: [PATCH 2/6] Address comments --- src/rcl_subscription_bindings.cpp | 32 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/rcl_subscription_bindings.cpp b/src/rcl_subscription_bindings.cpp index d1bf59d2..8228b1b9 100644 --- a/src/rcl_subscription_bindings.cpp +++ b/src/rcl_subscription_bindings.cpp @@ -38,9 +38,9 @@ Napi::Value RclTake(const Napi::CallbackInfo& info) { rcl_ret_t ret = rcl_take(subscription, msg_taken, nullptr, nullptr); if (ret != RCL_RET_OK && ret != RCL_RET_SUBSCRIPTION_TAKE_FAILED) { + std::string error_string = rcl_get_error_string().str; rcl_reset_error(); - Napi::Error::New(env, rcl_get_error_string().str) - .ThrowAsJavaScriptException(); + Napi::Error::New(env, error_string).ThrowAsJavaScriptException(); return Napi::Boolean::New(env, false); } @@ -109,9 +109,9 @@ Napi::Value CreateSubscription(const Napi::CallbackInfo& info) { expression.c_str(), argc, (const char**)argv, &subscription_ops); if (ret != RCL_RET_OK) { + std::string error_string = rcl_get_error_string().str; rcl_reset_error(); - Napi::Error::New(env, rcl_get_error_string().str) - .ThrowAsJavaScriptException(); + Napi::Error::New(env, error_string).ThrowAsJavaScriptException(); } if (argc) { @@ -255,9 +255,9 @@ Napi::Value SetContentFilter(const Napi::CallbackInfo& info) { } free(argv); } + std::string error_string = rcl_get_error_string().str; rcl_reset_error(); - Napi::Error::New(env, rcl_get_error_string().str) - .ThrowAsJavaScriptException(); + Napi::Error::New(env, error_string).ThrowAsJavaScriptException(); return env.Undefined(); } @@ -274,16 +274,16 @@ Napi::Value SetContentFilter(const Napi::CallbackInfo& info) { rcl_subscription_content_filter_options_fini(subscription, &options); if (ret != RCL_RET_OK) { + std::string error_string = rcl_get_error_string().str; rcl_reset_error(); - Napi::Error::New(env, rcl_get_error_string().str) - .ThrowAsJavaScriptException(); + Napi::Error::New(env, error_string).ThrowAsJavaScriptException(); return env.Undefined(); } if (fini_ret != RCL_RET_OK) { + std::string error_string = rcl_get_error_string().str; rcl_reset_error(); - Napi::Error::New(env, rcl_get_error_string().str) - .ThrowAsJavaScriptException(); + Napi::Error::New(env, error_string).ThrowAsJavaScriptException(); return env.Undefined(); } @@ -306,9 +306,9 @@ Napi::Value ClearContentFilter(const Napi::CallbackInfo& info) { subscription, "", 0, (const char**)nullptr, &options); if (ret != RCL_RET_OK) { + std::string error_string = rcl_get_error_string().str; rcl_reset_error(); - Napi::Error::New(env, rcl_get_error_string().str) - .ThrowAsJavaScriptException(); + Napi::Error::New(env, error_string).ThrowAsJavaScriptException(); return env.Undefined(); } @@ -317,16 +317,16 @@ Napi::Value ClearContentFilter(const Napi::CallbackInfo& info) { rcl_subscription_content_filter_options_fini(subscription, &options); if (ret != RCL_RET_OK) { + std::string error_string = rcl_get_error_string().str; rcl_reset_error(); - Napi::Error::New(env, rcl_get_error_string().str) - .ThrowAsJavaScriptException(); + Napi::Error::New(env, error_string).ThrowAsJavaScriptException(); return env.Undefined(); } if (fini_ret != RCL_RET_OK) { + std::string error_string = rcl_get_error_string().str; rcl_reset_error(); - Napi::Error::New(env, rcl_get_error_string().str) - .ThrowAsJavaScriptException(); + Napi::Error::New(env, error_string).ThrowAsJavaScriptException(); return env.Undefined(); } From cd2d89f53eb8f003a858ac25aeb01c57c003ea94 Mon Sep 17 00:00:00 2001 From: Minggang Wang Date: Wed, 14 Jan 2026 14:53:34 +0800 Subject: [PATCH 3/6] Address comments --- src/rcl_subscription_bindings.cpp | 37 +++++++++++++++++++------------ 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/rcl_subscription_bindings.cpp b/src/rcl_subscription_bindings.cpp index 8228b1b9..38fb7150 100644 --- a/src/rcl_subscription_bindings.cpp +++ b/src/rcl_subscription_bindings.cpp @@ -127,11 +127,14 @@ Napi::Value CreateSubscription(const Napi::CallbackInfo& info) { GetMessageTypeSupport(package_name, message_sub_folder, message_name); if (ts) { - THROW_ERROR_IF_NOT_EQUAL( - RCL_RET_OK, - rcl_subscription_init(subscription, node, ts, topic.c_str(), - &subscription_ops), - rcl_get_error_string().str); + rcl_ret_t ret = rcl_subscription_init(subscription, node, ts, topic.c_str(), + &subscription_ops); + if (ret != RCL_RET_OK) { + std::string error_msg = rcl_get_error_string().str; + rcl_reset_error(); + Napi::Error::New(env, error_msg).ThrowAsJavaScriptException(); + return env.Undefined(); + } auto js_obj = RclHandle::NewInstance( env, subscription, node_handle, [node, env](void* ptr) { @@ -139,8 +142,11 @@ Napi::Value CreateSubscription(const Napi::CallbackInfo& info) { reinterpret_cast(ptr); rcl_ret_t ret = rcl_subscription_fini(subscription, node); free(ptr); - THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret, - rcl_get_error_string().str); + if (ret != RCL_RET_OK) { + std::string error_msg = rcl_get_error_string().str; + rcl_reset_error(); + Napi::Error::New(env, error_msg).ThrowAsJavaScriptException(); + } }); return js_obj; @@ -346,9 +352,9 @@ Napi::Value GetContentFilter(const Napi::CallbackInfo& info) { rcl_ret_t ret = rcl_subscription_get_content_filter(subscription, &options); if (ret != RCL_RET_OK) { - Napi::Error::New(env, rcl_get_error_string().str) - .ThrowAsJavaScriptException(); + std::string error_msg = rcl_get_error_string().str; rcl_reset_error(); + Napi::Error::New(env, error_msg).ThrowAsJavaScriptException(); return env.Undefined(); } @@ -374,9 +380,9 @@ Napi::Value GetContentFilter(const Napi::CallbackInfo& info) { rcl_ret_t fini_ret = rcl_subscription_content_filter_options_fini(subscription, &options); if (fini_ret != RCL_RET_OK) { - Napi::Error::New(env, rcl_get_error_string().str) - .ThrowAsJavaScriptException(); + std::string error_msg = rcl_get_error_string().str; rcl_reset_error(); + Napi::Error::New(env, error_msg).ThrowAsJavaScriptException(); return env.Undefined(); } @@ -390,9 +396,12 @@ Napi::Value GetPublisherCount(const Napi::CallbackInfo& info) { RclHandle::Unwrap(info[0].As())->ptr()); size_t count = 0; - THROW_ERROR_IF_NOT_EQUAL( - rcl_subscription_get_publisher_count(subscription, &count), RCL_RET_OK, - rcl_get_error_string().str); + rcl_ret_t ret = rcl_subscription_get_publisher_count(subscription, &count); + if (ret != RCL_RET_OK) { + std::string error_msg = rcl_get_error_string().str; + rcl_reset_error(); + Napi::Error::New(env, error_msg).ThrowAsJavaScriptException(); + } return Napi::Number::New(env, count); } From d82195f448e5c1c9c6304e09fdd79de8576d252b Mon Sep 17 00:00:00 2001 From: Minggang Wang Date: Wed, 14 Jan 2026 15:27:57 +0800 Subject: [PATCH 4/6] Address comments --- test/test-type-description-service.js | 86 ++++++++++++++------------- 1 file changed, 45 insertions(+), 41 deletions(-) diff --git a/test/test-type-description-service.js b/test/test-type-description-service.js index 45af596b..c0e9f731 100644 --- a/test/test-type-description-service.js +++ b/test/test-type-description-service.js @@ -85,51 +85,55 @@ describe('type description service test suite', function () { }); it('Test type description service configured by parameter', function (done) { - exec( - 'ros2 param list /test_type_description_service', - (error, stdout, stderr) => { - if (error || stderr) { - done( - new Error( - `Test type description service configured by parameter failed. Error: ${error}, Stderr: ${stderr}` - ) - ); - return; - } - if (stdout.includes('start_type_description_service')) { - done(); - } else { - done( - new Error("'start_type_description_service' not found in stdout.") - ); + setTimeout(() => { + exec( + 'ros2 param list /test_type_description_service', + (error, stdout, stderr) => { + if (error || stderr) { + done( + new Error( + `Test type description service configured by parameter failed. Error: ${error}, Stderr: ${stderr}` + ) + ); + return; + } + if (stdout.includes('start_type_description_service')) { + done(); + } else { + done( + new Error("'start_type_description_service' not found in stdout.") + ); + } } - } - ); + ); + }, 1000); }); it('Test start_type_description_service parameter value', function (done) { - exec( - 'ros2 param get /test_type_description_service start_type_description_service', - (error, stdout, stderr) => { - if (error || stderr) { - done( - new Error( - `Test type description service configured by parameter failed. Error: ${error}, Stderr: ${stderr}` - ) - ); - return; - } - if (stdout.includes('Boolean value is: True')) { - done(); - } else { - console.log(stdout); - done( - new Error( - "'start_type_description_service param value' not found in stdout." - ) - ); + setTimeout(() => { + exec( + 'ros2 param get /test_type_description_service start_type_description_service', + (error, stdout, stderr) => { + if (error || stderr) { + done( + new Error( + `Test type description service configured by parameter failed. Error: ${error}, Stderr: ${stderr}` + ) + ); + return; + } + if (stdout.includes('Boolean value is: True')) { + done(); + } else { + console.log(stdout); + done( + new Error( + "'start_type_description_service param value' not found in stdout." + ) + ); + } } - } - ); + ); + }, 1000); }); }); From 4267f859acfa783ba4a33e3f5b12a1939ee5cf76 Mon Sep 17 00:00:00 2001 From: Minggang Wang Date: Wed, 14 Jan 2026 15:44:22 +0800 Subject: [PATCH 5/6] Address comments --- src/rcl_subscription_bindings.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/rcl_subscription_bindings.cpp b/src/rcl_subscription_bindings.cpp index 38fb7150..78bcd333 100644 --- a/src/rcl_subscription_bindings.cpp +++ b/src/rcl_subscription_bindings.cpp @@ -120,6 +120,11 @@ Napi::Value CreateSubscription(const Napi::CallbackInfo& info) { } free(argv); } + + if (ret != RCL_RET_OK) { + free(subscription); + return env.Undefined(); + } } } @@ -133,6 +138,7 @@ Napi::Value CreateSubscription(const Napi::CallbackInfo& info) { std::string error_msg = rcl_get_error_string().str; rcl_reset_error(); Napi::Error::New(env, error_msg).ThrowAsJavaScriptException(); + free(subscription); return env.Undefined(); } @@ -153,6 +159,7 @@ Napi::Value CreateSubscription(const Napi::CallbackInfo& info) { } else { Napi::Error::New(env, GetErrorMessageAndClear()) .ThrowAsJavaScriptException(); + free(subscription); return env.Undefined(); } } From c043b7812748ec61b2ee910d12257795d02a0f56 Mon Sep 17 00:00:00 2001 From: Minggang Wang Date: Wed, 14 Jan 2026 15:58:26 +0800 Subject: [PATCH 6/6] Address comments --- test/test-clock-event.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test-clock-event.js b/test/test-clock-event.js index 6e72a091..10276c00 100644 --- a/test/test-clock-event.js +++ b/test/test-clock-event.js @@ -42,7 +42,7 @@ describe('ClockEvent', function () { await event.waitUntilSteady(clock, until.nanoseconds); const end = Date.now(); - assert(end - start >= 1000); + assert(end - start >= 950); }); it('should wait until system time', async function () { @@ -55,7 +55,7 @@ describe('ClockEvent', function () { await event.waitUntilSystem(clock, until.nanoseconds); const end = Date.now(); - assert(end - start >= 1000); + assert(end - start >= 950); }); it('should wait until ros time', async function () { @@ -72,7 +72,7 @@ describe('ClockEvent', function () { await event.waitUntilRos(clock, until.nanoseconds); const end = Date.now(); - assert(end - start >= 1000); + assert(end - start >= 950); }); it('should set and clear event', function () {