search

pkgjs / parseargs

Polyfill of `util.parseArgs()`
Apache License 2.0
121 stars 9 forks source link

Unify custom error codes? #120

Closed shadowspawn closed 2 years ago

shadowspawn commented 2 years ago

The question came up again about whether we needed to introduce new error codes.

(As I said in reply, paraphrased) I think fine grained error codes are potentially useful for the author to build custom behaviour, but I also somewhat expect heavy customisation is more likely on top of strict:false where we (largely) don't throw anyway.

(As @bakkot said in reply, paraphrased) we do want rich messages to help the end user.

We could perhaps use a single custom error code so we can still supply rich message for end user, and so author can be confident it was a parsing error and not some completely unexpected error.

So instead of ERR_PARSE_ARGS_INVALID_OPTION_VALUE and ERR_PARSE_ARGS_UNKNOWN_OPTION and ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL, we could use single (say) ERR_PARSE_ARGS_INVALID_ARG.

shadowspawn commented 2 years ago

(I accidentally added this issue without a description. Now updated. This comment so some clue on email!)

ljharb commented 2 years ago

The program as well as the human needs to discern, so we absolutely shouldn't just use a single error code.

Error codes should generally map to messages 1:1, and a program should never have to parse a message to figure anything out.

shadowspawn commented 2 years ago

That raises an interesting point though, the error code is not enough alone for author to do much with. We might want to add some properties (in future).

An example would be an author catching ERR_PARSE_ARGS_UNKNOWN_OPTION and adding a message suggesting the most similar option.

$ util --hepl
Unknown option `--hepl` <--- error.message thrown by parseArgs
Did you mean `--help`?   <--- added by author, but how did they know it was `--hepl`?

For comparison, SystemError has a bunch of extra optional properties: https://nodejs.org/dist/latest-v16.x/docs/api/errors.html#class-systemerror

shadowspawn commented 2 years ago

There isn't a driver to do this from the upstream PR. Closing.