lib: refactor to use validateCallback#36609
Conversation
Trott
left a comment
There was a problem hiding this comment.
Definitely not for this PR, but for what it's worth, I'd be in favor getting rid of ERR_INVALID_CALLBACK entirely and using ERR_INVALID_ARG_TYPE instead. It is very difficult for me to imagine a situation where ERR_INVALID_CALLBACK is more useful to someone than ERR_INVALID_ARG_TYPE. On the other hand, it is easy for me to imagine a situation where someone has to check for both ERR_INVALID_ARG_TYPE and ERR_INVALID_CALLBACK but they don't know it and they're only checking for ERR_INVALID_ARG_TYPE. Node.js has too many error codes and they are often not specific in a way that is helpful to end users. ERR_INVALID_CALLBACK seems to be one of those cases.
Thanks for the reply, very valuable thought. node.js does have too many error codes, I was confused when I first opened the error.js file too.
However, |
|
Landed in 8cf5ae0...b00bb01 |
PR-URL: #36609 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: #36609 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Try replacing all duplicate checks with
validateCallback, Due to the scope of the changes, it may be necessary to run a benchmark.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes