3

One of the worst practices in programming is misusing exceptions to send messages.

This from the node manual for example:

> fsPromises.access(path[, mode])

> fsPromises.access('/etc/passwd', fs.constants.R_OK | fs.constants.W_OK)
> .then(() => console.log('can access'))
> .catch(() => console.error('cannot access'));

I keep seeing people doing this and it's exceptionally bad API design, excusing the pun.

This spec makes assumptions that not being able to access something is an error condition.

This is a mistaken assumption. It should return either true or false unless a genuine IO exception occurred.

It's using an exception to return a result. This is commonly seen with booleans and things that may or may not exist (using an exception instead of null or undefined).

If it returned a boolean then it would be up to me whether or not to throw an exception. They could also add a wrapper such as requireAccess for consistent error exceptions.

If I want to check that a file isn't accessible, for example for security then I need to wrap what would be a simple if statement with try catch all over the place. If I turn on my debugger and try to track any throw exception then they are false positives everywhere.

If I want to check ten files and only fail if none of them are accessible then again this function isn't suited.

I see this everywhere although it coming from a major library is a bit sad.

This may be because the underlying libraries are C which is a bit funky with error handling, there's at least a reason to sometimes squash errors and results together (IE, optimisation). I suspect the exception is being used because under the hood error codes are also used and it's trying to use throwing an exception to give the different codes but doesn't exist and bad permissions might not be an error condition or one requiring an exception.

Yet this is still the bane of my existence. Bad error handling everywhere including the other way around (things that should always be errors being warnings), in legacy code it's horrendous.

Comments
  • 2
    Fun fact:

    There's a deprecated function (exists()) that this with bools.
  • 1
    NodeJS' standard fs API also does this: https://nodejs.org/dist/... (except with callbacks instead of promises).

    Agree with your criticism, (eg PHP file_exists / python os.path.exists also returns true/ false & no error) although the blame should be placed on NodeJS, and not on a library author who follows the convention set by its environment =/
  • 0
    @kescherRant Sadly it's depreciated.

    There might be some reason, IE, if there's no explicit exists syscall.

    Though bash can manage it.
  • 2
    @webketje I'm not sure if assigning blame should have been that important, though trying to be fair or understand the cause is.

    There's two ways to write libraries and then somewhere in between. One is that you might generally try to pass through much from the underlying library as possible but you still have to make some changes for the differences and the other is to wrap it more extensively to produce higher level specifics.

    Given their methods seem to map to the common syscalls or standard lib calls underneath I get the impression they're going for passthru and trying to keep the conveniences lite.

    That said, even though it's a bit extra, normalising results and errors is still quite lite and I don't think it scope creeps much.

    Though this indeed is far broader than just node. Almost ever API/DB wrapper I use for example decides that not found is an error just as much as the NIC just fell out is and even further insists it has to be a HTTP error.
  • 0
    @webketje fspromises is actually just builtin "fs" module's promises property.
  • 1
    The whole idea of .access() is to return a file handler (in promises) or file descriptor (callback styled ones)
Add Comment