Path to stage 4! #2

Activity

1. bmeurer commented on Dec 1, 2020

   

   bmeurer

   on Dec 1, 2020

   More actions

   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

   👍React with 👍6Reacted by Ingvar Stepanyan, no, Hemanth HM, John Lenz, Chengzhong Wu and Justin Ridgewell
2. ljharb commented on Feb 26, 2021

   

   ljharb

   on Feb 26, 2021

   Member

   More actions

   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?
3. legendecas commented on Mar 2, 2021

   

   legendecas

   on Mar 2, 2021

   Last edited by legendecas

   Member

   More actions

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

   👍React with 👍1Reacted by Jordan Harband
4. ljharb commented on Mar 2, 2021

   

   ljharb

   on Mar 2, 2021

   Member

   More actions

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

   👍React with 👍1Reacted by Hemanth HM
5. syg commented on Mar 3, 2021

   

   syg

   on Mar 3, 2021

   Last edited by syg

   More actions

   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.

   👍React with 👍1Reacted by Chengzhong Wu
6. bakkot commented on Mar 3, 2021

   

   bakkot

   on Mar 3, 2021

   Member

   More actions

   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.

   👍React with 👍2Reacted by Jordan Harband and ExE Boss
7. syg commented on Mar 3, 2021

   

   syg

   on Mar 3, 2021

   More actions

   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 ??.
8. bakkot commented on Mar 3, 2021

   

   bakkot

   on Mar 3, 2021

   Member

   More actions

   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.

   👍React with 👍2Reacted by Jordan Harband and ExE Boss
9. ljharb commented on Mar 3, 2021

   

   ljharb

   on Mar 3, 2021

   Member

   More actions

   I asked for the steps to be in an AO - mainly because they were repeated three times and seemed easy to mess up.
10. syg commented on Mar 3, 2021

    

    syg

    on Mar 3, 2021

    More actions

    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.

    👍React with 👍1Reacted by Jordan Harband
11. syg commented on Mar 3, 2021

    

    syg

    on Mar 3, 2021

    More actions

    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.
12. bakkot commented on Mar 3, 2021

    

    bakkot

    on Mar 3, 2021

    Member

    More actions

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

    👍React with 👍1Reacted by Jordan Harband
13. syg commented on Mar 3, 2021

    

    syg

    on Mar 3, 2021

    More actions

    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.
14. bakkot commented on Mar 3, 2021

    

    bakkot

    on Mar 3, 2021

    Member

    More actions

    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.

    👍React with 👍2Reacted by Hemanth HM and Chengzhong Wu
15. syg commented on Mar 3, 2021

    

    syg

    on Mar 3, 2021

    More actions

    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.

    👍React with 👍2Reacted by Hemanth HM and Chengzhong Wu
16. jugglinmike commented on Mar 12, 2021

    

    jugglinmike

    on Mar 12, 2021

    More actions

    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.

    👀React with 👀1Reacted by Hemanth HM
17. ljharb commented on Mar 12, 2021

    

    ljharb

    on Mar 12, 2021

    Member

    More actions

    @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.
18. MadProbe commented on Mar 15, 2021

    

    MadProbe

    on Mar 15, 2021

    Last edited by MadProbe

    More actions

    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

    🎉React with 🎉4Reacted by Hemanth HM, Chengzhong Wu, Kenta Moriuchi and Rob Palmer

    🚀React with 🚀1Reacted by Hemanth HM
19. hemanth commented on Mar 15, 2021

    

    hemanth

    on Mar 15, 2021

    MemberAuthor

    More actions

    That's awesome news @MadProbe 🤓

    P.S: We are working on 262 tests.
20. Constellation commented on Mar 17, 2021

    

    Constellation

    on Mar 17, 2021

    Member

    More actions

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

    👍React with 👍5Reacted by Sergey Rubanov, Ross Kirsling, Chengzhong Wu, Rob Palmer and Nicolas HENRY

    🎉React with 🎉2Reacted by Hemanth HM and Chengzhong Wu

    🚀React with 🚀4Reacted by Hemanth HM, Chengzhong Wu, Pelle Wessman and Kenta Moriuchi
21. rkirsling commented on Mar 17, 2021

    

    rkirsling

    on Mar 17, 2021

    Member

    More actions

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

    👍React with 👍5Reacted by Chengzhong Wu, Yusuke Suzuki, Hemanth HM, Kenta Moriuchi and Jan Štola
22. avp commented on Mar 24, 2021

    

    avp

    on Mar 24, 2021

    More actions

    Landed in Hermes: facebook/hermes@56e7cda

    🎉React with 🎉3Reacted by Hemanth HM, Chengzhong Wu and Rob Palmer

    🚀React with 🚀1Reacted by Hemanth HM
23. BasixKOR commented on Jul 19, 2021

    

    BasixKOR

    on Jul 19, 2021

    Last edited by BasixKOR

    More actions

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

    I confirmed this on Firefox 91.0b4.

    🎉React with 🎉4Reacted by Nicolas HENRY, no, Chengzhong Wu and Pelle Wessman
24. Turbo87 commented on Sep 1, 2021

    

    Turbo87

    on Sep 1, 2021

    More actions

    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 🥳

    👍React with 👍2Reacted by Chengzhong Wu and Sung Jeon
25. BasixKOR commented on Nov 5, 2021

    

    BasixKOR

    on Nov 5, 2021

    More actions

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

    🚀React with 🚀1Reacted by Chengzhong Wu
26. saschanaz commented on Nov 5, 2021

    

    saschanaz

    on Nov 5, 2021

    Last edited by saschanaz

    More actions

    I guess this can be closed since it has already entered stage 4 and been merged to ecma262? tc39/ecma262#2356
27. legendecas commented on Nov 6, 2021

    

    legendecas

    on Nov 6, 2021

    Member

    More actions

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

    closed this as completedon Nov 6, 2021