Change serializer errors to use error codes #4464
Conversation
lib/nodejs/serializer.js
Outdated
| @@ -131,7 +134,12 @@ class SerializableEvent { | |||
| */ | |||
| constructor(eventName, originalValue, originalError) { | |||
| if (!eventName) { | |||
| throw new Error('expected a non-empty `eventName` string argument'); | |||
| throw createInvalidArgumentValueError( | |||
There was a problem hiding this comment.
if eventName is undefined (e.g., someone wrote new SerializableEvent()) then one could argue this is an invalid type (it should be a string; not undefined) instead of an invalid value. contrast that with passing '' for eventName--the type is correct but the value is invalid. in this case, we're checking for falsy, which is both a type and a value (because JavaScript)--and is intended--so I don't feel strongly that we need two different errors here. just wanted to mention it
further, these two types of errors are unlikely to be caused by an end-user--it's an error only Mocha contributors should ever encounter. we don't usually do this sort of exhaustive type-checking unless it's user input
There was a problem hiding this comment.
Makes sense to me. I wasn't tied to the two different error types, so I've gone ahead and changed this one to throw a type error too.
boneskull
added
area: node.js
|
calling this |
|
Thanks! |
Description of the Change
After the implementations of #3467 and #3666, there are a few instances in the code that do not use error codes yet. This PR addresses the instances in the serializer code and does not cover all remaining instances of lack of error codes (I mainly wanted to make sure I was on the right track here before trying to change as many remaining errors as possible).
Alternate Designs
N/A
Why should this be in core?
Using codes is a best practice like in Node.js, codes help identify errors as mocha errors at a glance
Benefits
Using codes is a best practice like in Node.js, codes help identify errors as mocha errors at a glance
Possible Drawbacks
None I'm aware of
Applicable issues
Contributes to #3656, semver-patch