Skip to content

Commit e5f93e0

Browse files
committed
Fix crashing when content filter not supported by RMW
1 parent 084bd4f commit e5f93e0

2 files changed

Lines changed: 48 additions & 2 deletions

File tree

lib/action/client.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,30 @@ class ActionClient extends Entity {
118118
this.qos.statusSubQosProfile
119119
);
120120

121+
// Probe that the RMW actually supports content filtering before enabling
122+
// the optimization. Some RMW implementations (e.g. FastDDS) abort the
123+
// process with SIGABRT instead of returning an error when content filter
124+
// operations are attempted on an unsupported subscription, so we cannot
125+
// rely on catching errors at first use like rclpy does.
126+
if (this._enableFeedbackMsgOptimization) {
127+
try {
128+
const probeSub = node.createSubscription(
129+
this._typeClass.impl.FeedbackMessage,
130+
actionName + '/_action/feedback',
131+
() => {}
132+
);
133+
const supported =
134+
typeof probeSub.isContentFilterSupported === 'function' &&
135+
probeSub.isContentFilterSupported();
136+
probeSub.destroy();
137+
if (!supported) {
138+
this._enableFeedbackMsgOptimization = false;
139+
}
140+
} catch {
141+
this._enableFeedbackMsgOptimization = false;
142+
}
143+
}
144+
121145
node._addActionClient(this);
122146
}
123147

test/test-action-client.js

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,28 @@ describe('rclnodejs action client', function () {
317317
typeof nativeLoader.actionConfigureFeedbackSubFilterAddGoalId ===
318318
'function';
319319

320+
// Probe whether the RMW actually supports content filtering, matching
321+
// the same check the ActionClient constructor performs.
322+
let isContentFilterSupported = false;
323+
before(function () {
324+
if (isFeedbackFilterSupported()) {
325+
try {
326+
const Fibonacci_ = rclnodejs.require(fibonacci);
327+
const probeSub = node.createSubscription(
328+
Fibonacci_.FeedbackMessage,
329+
'fibonacci/_action/feedback',
330+
() => {}
331+
);
332+
isContentFilterSupported =
333+
typeof probeSub.isContentFilterSupported === 'function' &&
334+
probeSub.isContentFilterSupported();
335+
probeSub.destroy();
336+
} catch {
337+
isContentFilterSupported = false;
338+
}
339+
}
340+
});
341+
320342
it('Test option defaults to false', function () {
321343
let client = new rclnodejs.ActionClient(node, fibonacci, 'fibonacci');
322344
assert.strictEqual(client._enableFeedbackMsgOptimization, false);
@@ -327,8 +349,8 @@ describe('rclnodejs action client', function () {
327349
let client = new rclnodejs.ActionClient(node, fibonacci, 'fibonacci', {
328350
enableFeedbackMsgOptimization: true,
329351
});
330-
// Only enabled when native API exists
331-
if (isFeedbackFilterSupported()) {
352+
// Only enabled when native API exists AND the RMW supports content filtering
353+
if (isFeedbackFilterSupported() && isContentFilterSupported) {
332354
assert.strictEqual(client._enableFeedbackMsgOptimization, true);
333355
} else {
334356
assert.strictEqual(client._enableFeedbackMsgOptimization, false);

0 commit comments

Comments
 (0)