Path to stage 4!

Question

Path to stage 4!

Closed this issue 3 years ago · 27 comments

Answer 1 · 2020-12-01T06:24:16.000Z

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

Answer 2 · 2021-02-26T16:45:03.000Z

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?

Answer 3 · 2021-03-02T03:21:20.000Z

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

Answer 4 · 2021-03-02T04:58:05.000Z

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

Answer 5 · 2021-03-03T02:17:06.000Z

Editor review

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

the behavior is ultimately DefinePropertyOrThrow, which is not what the name suggests. It reads like it's analogous to CreateDataProperty, which doesn't throw.
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.

Answer 6 · 2021-03-03T02:21:16.000Z

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.

Answer 7 · 2021-03-03T02:37:04.000Z

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 ??.

Answer 8 · 2021-03-03T02:43:37.000Z

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.

Answer 9 · 2021-03-03T06:51:07.000Z

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

Answer 10 · 2021-03-03T17:41:24.000Z

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.

Answer 11 · 2021-03-03T17:44:58.000Z

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.

Answer 12 · 2021-03-03T18:04:39.000Z

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".

Answer 13 · 2021-03-03T18:10:57.000Z

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.

Answer 14 · 2021-03-03T18:16:54.000Z

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.

Answer 15 · 2021-03-03T18:49:41.000Z

Yeah... I'm fine with this behavior to be clear. I'm lamenting we just don't have a general design principle for whether to conflate undefined with absence.

Answer 16 · 2021-03-12T20:53:47.000Z

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 legendecas has made the name change you requested, but now, the proposal introduces an "OrThrow" method that never throws. The prior name and use of Assert seems more direct to me. Would your original motivation be addressed if this were instead defined as an infallible abstract operation? Here's a patch demonstrating what I have in mind.

Answer 17 · 2021-03-12T23:06:49.000Z

@jugglinmike it doesn't throw with its current usage, but it could throw if called from other places, so "OrThrow" seems like the right place to me.

Answer 18 · 2021-03-15T18:26:25.000Z

ChakraCore implementation

My PR(chakra-core/ChakraCore#6621) with implementation (unflagged) of this proposal was merged to master branch of ChakraCore yesterday at time of writing of this comment

Note: Microsoft Edge no longer uses ChakraCore and now it's an open-source community project
Edit: AggregateError is not implemented in ChakraCore yet (there is a PR - chakra-core/ChakraCore#6301, - but not updated to complete the implementaion of this proposal and merged), so implementation is not full yet

Answer 19 · 2021-03-15T19:26:05.000Z

That's awesome news @MadProbe 🤓

P.S: We are working on 262 tests.

Answer 20 · 2021-03-17T12:39:44.000Z

WebKit/WebKit@b03c4f4 @rkirsling landed WebKit/JSC implementation.

Answer 21 · 2021-03-17T20:57:23.000Z

Note for implementers:

According to the WASM JS API spec...

The constructor and properties of WebAssembly errors is as specified for NativeError.

So don't forget to update the WebAssembly.{CompileError, LinkError, RuntimeError} constructors too. 🙂

Answer 22 · 2021-03-24T20:07:23.000Z

Landed in Hermes: facebook/hermes@56e7cda

Answer 23 · 2021-07-19T10:21:08.000Z

Landed in Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1679653

I confirmed this on Firefox 91.0b4.

Answer 24 · 2021-09-01T14:15:33.000Z

FWIW Sentry has implemented support for a cause property back in late 2018 (see getsentry/sentry-javascript#1401) and from what I've seen so far it should be compatible with this proposal 🥳

Answer 25 · 2021-11-05T20:21:32.000Z

Labded in TypeScript with "target": "ES2022" microsoft/TypeScript#46291

Answer 26 · 2021-11-05T20:53:45.000Z

I guess this can be closed since it has already entered stage 4 and been merged to ecma262? tc39/ecma262#2356

Answer 27 · 2021-11-06T07:55:27.000Z

Closing as the proposal has reached consensus on stage 4. Thanks to everyone for the help!

Path to stage 4!

Stage 4

Stage 3

Stage 2

Editor review

Delegate review

ChakraCore implementation