Skip to content

Commit 6559034

Browse files
committed
Address comments
1 parent 74c971c commit 6559034

2 files changed

Lines changed: 38 additions & 25 deletions

File tree

lib/event_handler.js

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@
1616

1717
const rclnodejs = require('./native_loader.js');
1818
const DistroUtils = require('./distro.js');
19-
const { OperationError } = require('./errors.js');
19+
const {
20+
OperationError,
21+
RangeValidationError,
22+
TypeValidationError,
23+
} = require('./errors.js');
2024
const Entity = require('./entity.js');
2125

2226
/**
@@ -64,8 +68,10 @@ const SubscriptionEventType = {
6468
* @param {number} eventType - A {@link PublisherEventType} value.
6569
* @return {boolean} True if the event type is supported by the active RMW
6670
* implementation, false otherwise.
67-
* @throws {OperationError} if invoked on a ROS distro older than Rolling, or
68-
* if eventType is not a valid {@link PublisherEventType} value.
71+
* @throws {OperationError} if invoked on a ROS distro older than Rolling.
72+
* @throws {TypeValidationError} if eventType is not a number.
73+
* @throws {RangeValidationError} if eventType is not a valid
74+
* {@link PublisherEventType} value.
6975
*/
7076
function isPublisherEventTypeSupported(eventType) {
7177
if (typeof rclnodejs.isPublisherEventTypeSupported !== 'function') {
@@ -81,15 +87,19 @@ function isPublisherEventTypeSupported(eventType) {
8187
}
8288
);
8389
}
84-
if (
85-
typeof eventType !== 'number' ||
86-
!Object.values(PublisherEventType).includes(eventType)
87-
) {
88-
throw new OperationError(`Invalid PublisherEventType value: ${eventType}`, {
89-
code: 'INVALID_ARGUMENT',
90+
if (typeof eventType !== 'number') {
91+
throw new TypeValidationError('eventType', eventType, 'number', {
9092
entityType: 'publisher event type',
9193
});
9294
}
95+
if (!Object.values(PublisherEventType).includes(eventType)) {
96+
throw new RangeValidationError(
97+
'eventType',
98+
eventType,
99+
'one of PublisherEventType values',
100+
{ entityType: 'publisher event type' }
101+
);
102+
}
93103
return rclnodejs.isPublisherEventTypeSupported(eventType);
94104
}
95105

@@ -102,8 +112,10 @@ function isPublisherEventTypeSupported(eventType) {
102112
* @param {number} eventType - A {@link SubscriptionEventType} value.
103113
* @return {boolean} True if the event type is supported by the active RMW
104114
* implementation, false otherwise.
105-
* @throws {OperationError} if invoked on a ROS distro older than Rolling, or
106-
* if eventType is not a valid {@link SubscriptionEventType} value.
115+
* @throws {OperationError} if invoked on a ROS distro older than Rolling.
116+
* @throws {TypeValidationError} if eventType is not a number.
117+
* @throws {RangeValidationError} if eventType is not a valid
118+
* {@link SubscriptionEventType} value.
107119
*/
108120
function isSubscriptionEventTypeSupported(eventType) {
109121
if (typeof rclnodejs.isSubscriptionEventTypeSupported !== 'function') {
@@ -119,16 +131,17 @@ function isSubscriptionEventTypeSupported(eventType) {
119131
}
120132
);
121133
}
122-
if (
123-
typeof eventType !== 'number' ||
124-
!Object.values(SubscriptionEventType).includes(eventType)
125-
) {
126-
throw new OperationError(
127-
`Invalid SubscriptionEventType value: ${eventType}`,
128-
{
129-
code: 'INVALID_ARGUMENT',
130-
entityType: 'subscription event type',
131-
}
134+
if (typeof eventType !== 'number') {
135+
throw new TypeValidationError('eventType', eventType, 'number', {
136+
entityType: 'subscription event type',
137+
});
138+
}
139+
if (!Object.values(SubscriptionEventType).includes(eventType)) {
140+
throw new RangeValidationError(
141+
'eventType',
142+
eventType,
143+
'one of SubscriptionEventType values',
144+
{ entityType: 'subscription event type' }
132145
);
133146
}
134147
return rclnodejs.isSubscriptionEventTypeSupported(eventType);

test/test-event-handle.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,19 +113,19 @@ describe('Event type is supported - native binding available', function () {
113113
it('isPublisherEventTypeSupported rejects invalid event types', function () {
114114
assert.throws(() => {
115115
isPublisherEventTypeSupported(-1);
116-
}, /Invalid PublisherEventType value/);
116+
}, /Value '-1' for 'eventType' is out of range: one of PublisherEventType values/);
117117
assert.throws(() => {
118118
isPublisherEventTypeSupported('matched');
119-
}, /Invalid PublisherEventType value/);
119+
}, /Invalid type for 'eventType': expected number, got string/);
120120
});
121121

122122
it('isSubscriptionEventTypeSupported rejects invalid event types', function () {
123123
assert.throws(() => {
124124
isSubscriptionEventTypeSupported(999);
125-
}, /Invalid SubscriptionEventType value/);
125+
}, /Value '999' for 'eventType' is out of range: one of SubscriptionEventType values/);
126126
assert.throws(() => {
127127
isSubscriptionEventTypeSupported(undefined);
128-
}, /Invalid SubscriptionEventType value/);
128+
}, /Invalid type for 'eventType': expected number, got undefined/);
129129
});
130130
});
131131

0 commit comments

Comments
 (0)