Path to stage 4!

[hemanth]

Stage 4

• committee approval
• implement in two browsers
  • node
  • Chrome Canary / v8
    • Chrome (flagged)
    • Chrome (unflagged)
  • Firefox / SpiderMonkey
    • Firefox Beta
    • Firefox (unflagged)
  • Safari / Webkit / JSC
    • Safari (unflagged)
    • Safari Technical Preview
  • Edge / Chakra
    • Edge (unflagged)
  • TypeScript
  • Hermes
• ecma262 PR approved
• prepare ecma262
• merge test262 tests
• write test262 tests

Stage 3

• committee approval
• spec editor signoff
• spec reviewer signoff from @ljharb and @codehag
• receive implementor feedback

Stage 2

• receive developer feedback
• committee approval
• spec text written
• spec reviewers selected

[bmeurer]

From a Chrome DevTools perspective, Error.prototype.cause seems reasonable to me. I was already thinking about ways to retain the existing (internal) stack trace on Error objects when programs rethrow errors, i.e. like in this case:

try {
  ...
} catch (e) {
  // some code
  throw e;
}

Here we would keep track of the original stack trace on e and show that similar to the way we show async stack traces in DevTools front-end. The addition of Error.prototype.cause would make it possible to extend this logic to cases where the developer not simply rethrows the current exception, but wraps it in a new Error object.

cc @RReverser

[ljharb]

Since this is on the agenda for stage 3, it seems like the OP of this issue needs to be updated, and a number of open issues still need to be addressed?

Additionally, who are the designated reviewers?

[legendecas]

@ljharb do you mean the reviewers have been discussed at #15? If so, I believe the reviewers are you and @codehag.

[ljharb]

Indeed :-) i wanted to make sure that plenary consensus on selecting reviewers was recorded in this repo.

[syg]

Editor review

I find the CreateNonEnumerableDataProperty unnecessary and confusing. It's confusing because:

1. the behavior is ultimately DefinePropertyOrThrow, which is not what the name suggests. It reads like it's analogous to CreateDataProperty, which doesn't throw.
2. The name doesn't tell me what the [[Configurable]] and [[Writable]] values are.

I'd prefer that the steps be inlined into their few call sites, it's only two steps anyway.

Delegate review

In InstallErrorCause, what's the rationale for doing both a [[Has]] and a [[Get]]? Most option bags (and the ones in 402), AFAIK, do a single [[Get]] and check if the value is undefined. I'd prefer a single [[Get]] that checks for undefined.

[bakkot]

In InstallErrorCause, what's the rationale for doing both a [[Has]] and a [[Get]]? Most option bags (and the ones in 402), AFAIK, do a single [[Get]] and check if the value is undefined. I'd prefer a single [[Get]] that checks for undefined.

That's something I asked for. It's possible to throw undefined, so it should be possible to create an error whose cause is undefined.

[syg]

That's something I asked for. It's possible to throw undefined, so it should be possible to create an error whose cause is undefined.

I see, thanks for the explanation. I guess it's a legit use case, but it seems pretty edge-casey to me.

It also means that since "not having a cause" and "having a nullish cause" are supposed to be distinguished states, consumers can't consume this in the "obvious" way with nullish coalescing ??.

[bakkot]

consumers can't consume this in the "obvious" way with nullish coalescing ??.

That depends on whether the consumers care to distinguish those cases. I suspect most won't; I just think we shouldn't make it impossible to do so.

[ljharb]

I asked for the steps to be in an AO - mainly because they were repeated three times and seemed easy to mess up.

[syg]

I asked for the steps to be in an AO - mainly because they were repeated three times and seemed easy to mess up.

If you think the refactoring is important, I think it should have an "OrThrow" appended to it. Otherwise I'm still happier with it just inlined.

[syg]

That depends on whether the consumers care to distinguish those cases. I suspect most won't; I just think we shouldn't make it impossible to do so.

That's what I initially thought, but then it seemed incorrect to me for the consumer to not care if the producer uses the cause to mean "was this a caught-and-rethrown error or not". Concretely, if the producer decided to distinguish, DevTools UI may be presenting misleading information if it doesn't.

[bakkot]

Well, the devtools UI should probably also distinguish, yes. But certainly in my own code I plan to write if (error.cause) {...} and complain to upstream libraries if that breaks because they're throwing a non-object value.

To put it another way: producers should not ever throw undefined, and it's fine for user code to be written as if that never happens. But people can do things they shouldn't, and the language should try to behave reasonably even in those cases: here, that means that if someone does catch (e) { throw new Error(msg, { cause: e }) }, they should always end up with an error with a cause, even in the case of someone else doing something they shouldn't.

I don't think we should extrapolate from "the language should accommodate people doing a weird thing" to "user code in general should accommodate that weird thing".

[syg]

I don't think we should extrapolate from "the language should accommodate people doing a weird thing" to "user code in general should accommodate that weird thing".

Agreed.

For this specific case, I don't feel very strongly at all. In general, there is unfortunately precedent in the language where undefined is already conflated with "absence", like optional parameters (despite presence of arguments.length). So another part of my question here is why accommodate here.

[bakkot]

Well:

if someone does catch (e) { throw new Error(msg, { cause: e }) }, they should always end up with an error with a cause

I don't have much of an argument beyond that.