Skip to content

Latest commit

 

History

History
89 lines (48 loc) · 7.09 KB

02-08.md

File metadata and controls

89 lines (48 loc) · 7.09 KB

February 8, 2021 Incubator Call Notes

Attendees:

  • Hemanth HM (HHM)
  • Chenzhong Wu (CZW)
  • Kevin Gibbons (KG)
  • Shu-yu Guo (SYG)
  • Benedikt Meurer (BMR)
  • Richard Gibson (RGN)

Error cause

HHM: Previously questions were asked and developers weren't sure how Error.cause would be surfaced up. We thought we could expose that in a playground-like thing like the Temporal proposal has.

BMR: I was under the impression that if we have this we'd display this along with async stack traces.

HHM: I'm trying to share a quick screenshot.

HHM: One part of the question is how devtools would use it, another part is how it would help developers.

KG: I'm confused. The whole point of this proposal is to give the root cause. It would help developers because errors would now have a root cause. It's the whole point of the proposal, not sure how this is a question.

HHM: We also have a question of grouping. If developers throw errors with the same root cause, would it help developers to group certain sets of causes?

KG: Not sure I understand the question. My experience is that an error gets thrown and gets caught in one place, perhaps wrapped, or percolates to the top and you get an uncaught exception. You don't get multiple errors going on. Only exception is when you have a rejected promise which s awaited in multiple places. Also not something I understood to be part of the intent of the proposal. Having a bunch of errors and wanting to group them doesn't seem to be part of this proposal.

HHM: (Shares screen to show what he means)

BMR: This is just Error.stack more or less. DevTools just does something magical on top but this is just like error.stack.

CZW: There's no modifications to Error.stack.

KG: There are two separate stacks here, each has a separate stack trace.

BMR: I guess that should be doable with some additional plumbing. We generally try to get error.stack and devtools in sync, but not always possible.

CZW: (Shows how the node repl was modified.) Previously we had a request for buy-in from devtools, so we made a simple and naive demo on node and devtools frontend to bring our own view of the proposal in the real world and help us identify the cause of the exception.

BMR: We've also talked about this on the devtools V8 side a bit. Orthogonal to this proposal, if the primary use case is to have it in the callstack, it might be possible to get this without changing the language at all. The engine under the hood has the old stack trace. When you throw from a catch block, we know how you got there, and we can attach the old trace under the hood. We even have a bug open for this: https://bugs.chromium.org/p/chromium/issues/detail?id=1167090 We haven't implemented it but we're pretty sure it's doable. Question is what you want to get out of the cause. If you want the original error then yes you'd need this. If you just want to display the stack we might be able to do this without language changes. I'm surprised you don't want to add this to Error.stack because it might be used for the monitoring software used in the wild.

KG: Monitoring software could start looking at this property as well. The cause isn't really part of the stack. It's not another thing on top of the stack trace, it's a separate dimension.

BMR: Yeah but same could be said for async stack traces.

KG: It is true mechanically for async parts you don't have the trace around anymore. But the mental model of programmers is to pretend it's synchronous, and if you pretend it's synchronous, it's a single stack.

BMR: The cause is more powerful since you can stash the original error. Just saying if the only use case is the extra stack info devtools can probably do it another way.

CZW: How can node or devtools inspect the cause property if it's not in the language itself?

SYG: To summarize Benedikt, why would you want to inspect the cause property? If it's to actually get the original object, you'd need to change the language. But if your actual use case is these additional stack traces when printing, i.e. if your root cause is always going to be the error you caught and rethrow in a catch block, that can happen under the hood so that devtools displays the stack in the re-thrown error.

BMR: Yes. Even if you throw a new error in the catch block.

CZW: We don't want to modify the original error but attach context to the new error. This expressivity is already available to devtools: We don't want to modify the caught exception. In node the error objects have codes, and we want to preserve those in the chain of error causes.

BMR: So you want to have this in the callstack, and in the callstack we can display this like the async stack traces. You might have one or more causes, and after all the causes we'll show the async parts?

CZW: I think we'd prefer the info of the error that's just thrown first, and then show one of the cause and show the number of additional causes in the chain. That's what we believe we could do: the info about the error itself would go first.

Benedikt: What kind of property are we talking about? Regular data property?

CZW: Regular data property.

BMR: Feel free to comment on the preserve-stack traces bug. This was brought up by Ingvar Stepanyan because it's a problem for wasm. Wasm stack has to wrap all thrown errors.

HHM: We're thinking of adding a playground on the GH landing page like Temporal and gather developer feedback.

SYG: Sounds good, but be aware if the feedback is on how the cause property is displayed, that's a product decision for Node or the various devtools and not the language. Please be mindful to relay that feedback back to those various teams.

CZW: The shape of the constructor is now that it takes an options bag with a cause property, so that it works better with DOMException and can be extended in the future. We also changed all the built-in constructors so that they take this new parameter.

KG: Sounds good to me. Domenic suggested this.

SYG: Sounds good to me also.

SYG: We should also consider whether we want to be restrictive about the properties allowed on this bag. If all unknown properties are ignored, then platforms might start using other properties for their own purposes, and then we couldn't use them. The import assertions conclusion might be relevant here.

KG: The import assertions conclusion is we throw. I think the use case here is different that we can get away with being less restrictive here.

CZW: We'll bring it up at the next meeting. Module assertions is more like a declarative pattern and Error constructors might have options not immediately constructed in place. Might have unknown properties to us but known to developers. I personally wouldn't lean into the throwing pattern.

KG: I agree with you.

SYG: Me too, for more blatant reasons as a browser we might want to reserve the right to use other option properties.

SYG: The next step here would be to bring it back to committee. The "how restrictive should the options bag be" question can't be settled in an incubator call. And if you get feedback from another browser devtools team we should be good to go for stage 3.

KG: Yes, please get feedback from another devtools team, either Safari or Firefox, before bringing it for stage 3.