RobotWebTools / rclnodejs / 22705166217

86%

push

github

Fix: resolve logic bugs, resource leaks, and error handling issues in JavaScript lib/ (#1410)

Comprehensive review and fix of all JavaScript files under `lib/` that implement
the rclnodejs client library. Eight issues were identified and resolved.

### 1. Incorrect `instanceof` operator precedence (`lib/node.js`)

Changed `(!parameter) instanceof Parameter` to `!(parameter instanceof Parameter)`.
The original expression evaluated `!parameter` to a boolean first, which is never
an instance of `Parameter` — the validation was always `false`, silently accepting
invalid objects as parameter overrides.

### 2. Broken chained comparison in `validType()` (`lib/parameter.js`)

Changed `PARAMETER_NOT_SET <= parameterType <= PARAMETER_STRING_ARRAY` to two
separate comparisons joined with `&&`. JavaScript evaluates chained comparisons
as `(a <= b) <= c`, where `(a <= b)` produces a boolean (0 or 1) — the result
was always `true`, allowing any numeric value to pass type validation.

### 3. Missing `else if` in `_validArray()` (`lib/parameter.js`)

Changed three standalone `if` blocks to `else if` in the array element type
resolution. The conditions are mutually exclusive by design; without `else if`,
later branches could overwrite a correctly resolved `arrayElementType`.

### 4. Typo `_loggger` → `_logger` (`lib/lifecycle_publisher.js`)

Fixed triple-g typo in both the property declaration and usage site. The typo
was consistent so it didn't crash at runtime, but any code referencing the
standard `_logger` spelling would get `undefined`.

### 5. Duplicate getter/setter with `for...in` bug and missing unsubscribe (`lib/time_source.js`)

Removed the first duplicate `get/set isRosTimeActive` definition. It contained
two bugs: `for (const clock in ...)` iterated over array indices (strings), not
clock objects, making the property assignment a no-op; and the `else` branch
incorrectly set `this._node._clockSubscription` instead of
`this._clockSubscription`.

Additiona... (continued)

1388 of 1754 branches covered (79.13%)

Branch coverage included in aggregate %.

15 of 17 new or added lines in 7 files covered. (88.24%)

2868 of 3203 relevant lines covered (89.54%)

460.34 hits per line