Node: Readonly error.code has ecosystem impact.

[jdalton]

I know internal/errors usage is picking up in Node core so I thought I'd give a heads up on something I've seen in the wild. It looks like folks are adding error.code properties which will error with the internal/errors since error.code is readonly.

Is being readonly intentional?

[targos]

There was some discussion about that in the PR introducing internal/errors: https://github.com/nodejs/node/pull/11220#pullrequestreview-21835752

/cc @jasnell

[jasnell]

Yes, the read only was intentional in order to explicitly prevent blind overriding. I believe the property is still configurable tho. It's been a while tho so I may be wrong

[jdalton]

Yes, the read only was intentional in order to explicitly prevent blind overriding.

This applies to CJS code as well, right?

Is there a user-land scenario in mind, around augmenting Node specific errors, that you're wanting to prevent?

I'm wondering if the user-land code should have to worry about it. To user-land code an error object is an error object. It could be a syntax error, or some other kind of error, and now code will have to branch for Node specific error objects or handle all error objects as tricky ones (which I'd assume is not the usual case).

[jasnell]

I'm not against changing it. There is a side effect in that the code is used in the name property, so it's not entirely free

[jdalton]

There is a side effect in that the code is used in the name property, so it's not entirely free

The symbol prop is used in the name so .code and .name can be modified independently.

[evanlucas]

I would be +1 on changing. I've seen usage of overwriting Error#code some.

[jasnell]

What I mean is that changing code would make it out of sync with the name. We should decide whether those should be kept in sync automatically.

[jdalton]

We should decide whether those should be kept in sync automatically

Since the .code and .name props aren't tied together (.name doesn't rely on .code) I think it'd be fine to allow augmenting to be yolo (getting too fancy with syncing seems like unnecessary overhead).

[jasnell]

That's the answer I'd be hoping for ;)

[Trott]

Proposed quick fix in https://github.com/nodejs/node/pull/15694