Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Consider having cause always present #35

Closed
domenic opened this issue Jul 16, 2021 · 8 comments
Closed

Consider having cause always present #35

domenic opened this issue Jul 16, 2021 · 8 comments

Comments

@domenic
Copy link
Member

domenic commented Jul 16, 2021

The spec right now tries to use the existence of cause in the options bag to decide if cause should be present on the Error object.

This seems a bit weird, as we try in most parts of the language (and also the web platform) not to distinguish between "not present" and "present but undefined".

It also leads to annoying questions like, if I'm cloning an error, do I need to check for cause's presence? It'd be nicer if I could just do clone.cause = original.cause or, if I'm feeling fancy, Object.defineProperty(clone, "cause", Object.getOwnPropertyDescriptor(original, "cause")).

Finally, these sort of sometimes-one-shape-sometimes-another objects are pretty unusual for the ECMAScript spec. Although Errors are pretty unusual for the ECMAScript spec already, I think this extra bit of weirdness isn't necessary.

@domenic
Copy link
Member Author

domenic commented Jul 16, 2021

It's interesting to compare this issue with how message behaves. Several of my complaints (cloning, shifting-shape objects) hold for message as well, since 'message' in new Error() is false whereas 'message' in new Error("") is true.

However, message respects the semi-invariant that undefined and not-present should be treated the same: i.e., 'message' in new Error(undefined) is false, to match 'message' in new Error(). Coincidentally, I think that kind of consistency argument is the strongest of my complaints :).

@bakkot
Copy link
Contributor

bakkot commented Jul 16, 2021

('message' in new Error() is actually true because there is a message property on Error.prototype, but we were generally against having cause be present on Error.prototype.)

@domenic
Copy link
Member Author

domenic commented Jul 16, 2021

Ah, yep, substitute Object.getOwnPropertyDescriptor(x, 'message') !== undefined for the discriminator.

+1 on not having cause in Error.prototype.

@gibson042
Copy link

Clarification: Are you proposing that cause be a property on every error instance, or only on those created by invoking a constructor?

@domenic
Copy link
Member Author

domenic commented Jul 16, 2021

I don't have a strong preference on non-constructor-created error instances, since my strongest preference is for undefined to be treated the same as omitting the option in the constructor.

Having it be on all instances would be nicer for the cloning and shape-shifting object complaints, so I think it would be slightly better. But as noted above message already makes Error objects non-uniform so it wouldn't be a big win.

@gibson042
Copy link

I think all host-created error instances have a message property, even though that doesn't seem to be a normative requirement. The fact that constructors can create instances without one is a curiosity, but configurable properties by their very nature are unreliable anyway (e.g., delete err.message works). I would hope for as much consistency between host-created and constructed instances as possible, and that is possible to achieve regardless of whether or not the latter always start with a cause property (although if they do, it will make it practically impossible to distinguish the edge-case undefined cause from absent cause—which may not be considered sufficiently important to address).

@legendecas
Copy link
Member

Like said in the thread, it will be quite awkward if we can not distinguish if a cause is present or not.

IIUC, the problem here is that the error objects are not "fixed" shapes in various constructing patterns. As mentioned in the thread, the "message" property is already not in "fixed" shapes.

One alternative to the problem could be adding another boolean field hasCause to indicate the presence of the cause. In this way, we can always set the property cause to be either the value (maybe undefined too) in the option bag and hasCause to be true, or just undefined but hasCause to be false. However, this pattern is somewhat not "consistent" with the property message as Object.hasOwn(new Error(), 'message') === false. That is to say, in exact cloning, it has to check the existence of the message or the shape of the result of cloning will diverge from the original objects.

@legendecas
Copy link
Member

Closing as the proposal has reached consensus to stage 4.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants