Conflict on Enumerable: false

[AMorgaut]

If Enumerable is false:

• It will break current behavior of JSON.stringify(), for ... in, Object.keys(), Object.values(), and Object.entries() on errors with "manualy" set code properties.
• It will limit one of the key advantage (in my opinion) of this code property as it won't be propagated in logs by default (the error message only is propagated because developers painfuly learned to extract it manually instead of relying on default JSON stringify behavior)

We may have a look on how the code property is used in the mentioned libraries, but it is impossible to know how it is used by those library consumers worlwide, not even talking about application level defined code properties

Even if the purpose is good, I do think it is very ambitious to try add constraints on a such commonly used Error property name and probably incompatible with the W3C mantra "Don't break the Web", maybe another property name would be more appropriate

Conflict

Current behavior

const err = new Error('foo')
err.code = 'ERR_FOO' 
// or const err = Object.assign(new Error('foo'), { code: 'ERR_FOO' })
JSON.stringify(err) // '{"code":"ERR_FOO"}'

Behavior with Enumerable false

const err = new Error('foo')
Object.defineProperty(err, 'code', { value: 'ERR_FOO', enumerable: false })
JSON.stringify(err) // '{}'

[ljharb]

Why would it break them? Manually set properties don't inherit non-enumerability.

var e = new Error();
e.name // "Error"
JSON.stringify(e) // "{}"
e.name = 'foo';
JSON.stringify(e) // "{\"name\":\"foo\"}"

in other words, it'd only be non-enumerable in that scenario if you chose it, in which case, omitting it from JSON.stringify is the desired behavior.

[AMorgaut]

Well, I didn't search deeply but I see no existing app level or library level usage of the code property using Object.defineProperty(err, 'code', { value: 'ERR_FOO', enumerable: false }) to make it non enumerable.

While in the current design, the proposed code property is specified as this:

• Defined on instances, not on Error.prototype
• Type: any value (not restricted to strings, consistent with cause)
• Default: property is not present when not provided ('code' in err is
  false), consistent with cause
• Enumerable: false (consistent with cause and message)
• Writable: true (consistent with other Error properties)
• Configurable: true

So any error created using the proposed signature:

new Error("something went wrong", { code: "ERR_SOMETHING" })
new TypeError("expected string", { code: "ERR_INVALID_ARG_TYPE", cause: original })

Will have a different behavior than the other ones when managed from a catch block (which can receive errors thrown from any source)

Meaning that, as an example, if an existing catch block exposes the error code through JSON.stringify() (for either a log or an HTTP error response), it may still get it from its legacy errors but not for more recent ones.

Mitigation

As this behavior will be linked to the usage of a dedicated signature, it might be ok to just highlight such behavior in the documentation as a warning notice.

I think it is important to highlight it as the usage of the new signature could occur in different codebases than the code managing the error.

Let say tomorrow the firebase-js-sdk updates its FirebaseError class

from

export class FirebaseError extends Error {
  constructor(
    readonly code: string, // enumerable own code property 
    message: string,
    public customData?: Record<string, unknown>
  ) {
    super(message);
    /** other code **/
  }
}

to

export class FirebaseError extends Error {
  constructor(
    code: string, // string parameter
    message: string,
    public customData?: Record<string, unknown>
  ) {
    super(message, { code }); // the code property becomes non enumerable
    /** other code **/
  }
}

cf: firebase-js-sdk/packages/util/src/errors.ts#L80

Apps using it should see it in the Firebase SDK updated version through :

• a breaking change mention in its changelog
• a major version update to respect semantic versioning

Hopefuly, a project like the firebase SDK would take care of it, but without clearly mentionning it as a warning notice in the API documention (MDN adn other), it can very easily be missed

Note:

• Axios even force the message property to be enumerable as well: AxiosError.js#L113

[ljharb]

We don't determine what MDN lists, but you're certainly welcome to suggest that to them when they document it.

Axios has a comment there that indicates it's to have back compat with old versions of axios - not that it's the better choice.