Skip to content

Commit e610950

Browse files
authored
Leverage rcl_arguments_get_unparsed_ros() to get unsupported args (#1342)
This PR enhances ROS argument validation by leveraging the RCL API function `rcl_arguments_get_unparsed_ros()` to detect and report unknown ROS arguments. The implementation replaces the simple boolean check `HasUnparsedROSArgs()` with a more comprehensive `ThrowIfUnparsedROSArgs()` function that provides detailed error messages listing the specific unknown arguments. Key changes: - Implemented `ThrowIfUnparsedROSArgs()` to detect and report unparsed ROS arguments with detailed error messages - Added test coverage for unknown ROS argument detection during both context initialization and node creation - Integrated the new validation into both `rcl_init()` and `rcl_node_init()` code paths Fix: #1330
1 parent faaa2d4 commit e610950

5 files changed

Lines changed: 118 additions & 4 deletions

File tree

src/rcl_context_bindings.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,11 @@ Napi::Value Init(const Napi::CallbackInfo& info) {
7373
rcl_init(argc, argc > 0 ? argv : nullptr, &init_options, context),
7474
rcl_get_error_string().str);
7575

76+
ThrowIfUnparsedROSArgs(env, jsArgv, context->global_arguments);
77+
if (env.IsExceptionPending()) {
78+
return env.Undefined();
79+
}
80+
7681
THROW_ERROR_IF_NOT_EQUAL(
7782
RCL_RET_OK, rcl_logging_configure(&context->global_arguments, &allocator),
7883
rcl_get_error_string().str);

src/rcl_node_bindings.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,17 @@ Napi::Value CreateNode(const Napi::CallbackInfo& info) {
193193
rcl_arguments_t arguments = rcl_get_zero_initialized_arguments();
194194
rcl_ret_t ret =
195195
rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &arguments);
196-
if ((ret != RCL_RET_OK) || HasUnparsedROSArgs(arguments)) {
196+
if (ret != RCL_RET_OK) {
197197
Napi::Error::New(env, "failed to parse arguments")
198198
.ThrowAsJavaScriptException();
199199
return env.Undefined();
200200
}
201201

202+
ThrowIfUnparsedROSArgs(env, jsArgv, arguments);
203+
if (env.IsExceptionPending()) {
204+
return env.Undefined();
205+
}
206+
202207
RCPPUTILS_SCOPE_EXIT({
203208
if (RCL_RET_OK != rcl_arguments_fini(&arguments)) {
204209
Napi::Error::New(env, "failed to fini arguments")

src/rcl_utilities.cpp

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121

2222
#include <cstdio>
2323
#include <memory>
24+
#include <rcpputils/scope_exit.hpp>
25+
// NOLINTNEXTLINE
2426
#include <string>
2527

2628
namespace {
@@ -344,9 +346,51 @@ void FreeArgs(char** argv, size_t argc) {
344346
}
345347
}
346348

347-
bool HasUnparsedROSArgs(const rcl_arguments_t& rcl_args) {
349+
void ThrowIfUnparsedROSArgs(Napi::Env env, const Napi::Array& jsArgv,
350+
const rcl_arguments_t& rcl_args) {
348351
int unparsed_ros_args_count = rcl_arguments_get_count_unparsed_ros(&rcl_args);
349-
return unparsed_ros_args_count != 0;
352+
353+
if (unparsed_ros_args_count < 0) {
354+
Napi::Error::New(env, "Failed to count unparsed arguments")
355+
.ThrowAsJavaScriptException();
356+
return;
357+
}
358+
if (0 == unparsed_ros_args_count) {
359+
return;
360+
}
361+
362+
rcl_allocator_t allocator = rcl_get_default_allocator();
363+
int* unparsed_indices_c = nullptr;
364+
rcl_ret_t ret =
365+
rcl_arguments_get_unparsed_ros(&rcl_args, allocator, &unparsed_indices_c);
366+
if (RCL_RET_OK != ret) {
367+
Napi::Error::New(env, "Failed to get unparsed arguments")
368+
.ThrowAsJavaScriptException();
369+
return;
370+
}
371+
372+
RCPPUTILS_SCOPE_EXIT({
373+
allocator.deallocate(unparsed_indices_c, allocator.state);
374+
});
375+
376+
std::string unparsed_args_str = "[";
377+
for (int i = 0; i < unparsed_ros_args_count; ++i) {
378+
int index = unparsed_indices_c[i];
379+
if (index < 0 || static_cast<size_t>(index) >= jsArgv.Length()) {
380+
Napi::Error::New(env, "Got invalid unparsed ROS arg index")
381+
.ThrowAsJavaScriptException();
382+
return;
383+
}
384+
std::string arg = jsArgv.Get(index).As<Napi::String>().Utf8Value();
385+
unparsed_args_str += "'" + arg + "'";
386+
if (i < unparsed_ros_args_count - 1) {
387+
unparsed_args_str += ", ";
388+
}
389+
}
390+
unparsed_args_str += "]";
391+
392+
Napi::Error::New(env, "Unknown ROS arguments: " + unparsed_args_str)
393+
.ThrowAsJavaScriptException();
350394
}
351395

352396
} // namespace rclnodejs

src/rcl_utilities.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ char** AbstractArgsFromNapiArray(const Napi::Array& jsArgv);
6363
// `AbstractArgsFromNapiArray` and `FreeArgs` must be called in pairs.
6464
void FreeArgs(char** argv, size_t argc);
6565

66-
bool HasUnparsedROSArgs(const rcl_arguments_t& rcl_args);
66+
void ThrowIfUnparsedROSArgs(Napi::Env env, const Napi::Array& jsArgv,
67+
const rcl_arguments_t& rcl_args);
6768

6869
} // namespace rclnodejs
6970

test/test-unparsed-args.js

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Copyright (c) 2025, The Robot Web Tools Contributors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
'use strict';
16+
17+
const assert = require('assert');
18+
const rclnodejs = require('../index.js');
19+
20+
describe('rclnodejs unparsed ROS args', function () {
21+
this.timeout(10 * 1000);
22+
23+
beforeEach(function () {
24+
rclnodejs.shutdown();
25+
});
26+
27+
afterEach(function () {
28+
rclnodejs.shutdown();
29+
});
30+
31+
it('should throw error when initializing context with unknown ROS args', async function () {
32+
const args = ['--ros-args', '--unknown-arg'];
33+
try {
34+
await rclnodejs.init(rclnodejs.Context.defaultContext(), args);
35+
assert.fail('Should have thrown an error');
36+
} catch (e) {
37+
assert.ok(e.message.includes('Unknown ROS arguments'));
38+
assert.ok(e.message.includes('--unknown-arg'));
39+
}
40+
});
41+
42+
it('should throw error when creating node with unknown ROS args', async function () {
43+
await rclnodejs.init();
44+
const args = ['--ros-args', '--unknown-arg'];
45+
try {
46+
rclnodejs.createNode(
47+
'test_node',
48+
'',
49+
rclnodejs.Context.defaultContext(),
50+
rclnodejs.NodeOptions.defaultOptions,
51+
args
52+
);
53+
assert.fail('Should have thrown an error');
54+
} catch (e) {
55+
assert.ok(e.message.includes('Unknown ROS arguments'));
56+
assert.ok(e.message.includes('--unknown-arg'));
57+
}
58+
});
59+
});

0 commit comments

Comments
 (0)