Discuss blocking on Zones proposal

[bmeck]

Moving here, as per: nodejs/CTC#166 (comment)

https://github.com/domenic/zones is a proposal that is currently being blocked by Node.

The proposal is something similar to domains and async_hooks in creating a way to pass data through asynchronous tasks.

The blocker has been the handling of errors with the proposal.

I would like to setup a meeting to discuss the blocking behavior now that async_hooks is landed.

[Trott]

@nodejs/async_hooks I guess?

[AndreasMadsen]

We can setup a meeting in the diagnostics WG.

Does this have any relation to angular/zone.js#795? /cc @JiaLiPassion

[JiaLiPassion]

@AndreasMadsen , thanks for cc me, the nodejs/CTC#166 (comment) seems about error handling of zone proposal, it is not directly related to angular/zone.js#795, but I would like to provide any help I can.

[bmeck]

@JiaLiPassion Is the following no longer a goal?

5. Error
   use process.unCaughtException to redirect to ZoneDelegate.handleError.

[JiaLiPassion]

@bmeck ,

1. use process.unCaughtException to redirect to ZoneDelegate.handleError is still a goal, to use async_hooks to implements zone, this is the only way I found to handle error.

2. currently the [WIP] feat(async): fix #740, use async hooks to wrap nodejs async api angular/zone.js#795 is stopped because I can't find a way to implement zone aware promise with current async_hooks API.

[mcollina]

I am 👎 on changing process.on('uncaughtException') default behavior: crashing the process. It should be possible to enable zones if someone wants to, but a lot of modules and applications out there are developed thinking that an error will crash the process.

Generically, I would prefer that zones would create a new context that does not influences anything that happens outside of it.

[JiaLiPassion]

@mcollina , thank you for your advice, currently, my thought is just use async_hooks to simulate original zone.js monkey-patch behavior.

• with zone.js monkey-patch version. zone.js intercept all async callback, so it will be able to catch all errors.

• with async_hooks which is just a notification not an interception, so it is not possible to catch any errors.

So currently I just thought maybe with process.on('unaughtException'), it is a way to do what currently zone.js does.

[AndreasMadsen]

Generically, I would prefer that zones would create a new context that does not influences anything that happens outside of it.

Unfortunately, we don't have a mechanism for that. Domains works by catching the error beforeunaughtException, likely zones will work that way too.

@JiaLiPassion if you want to experiment with it, you can overwrite process._fatalException or process.domain._errorHandler. it is not a public API so there are never any guarantees but process._fatalException especially is not something that is likely to change.

[JiaLiPassion]

@AndreasMadsen , thank you ! I will try it.

[MylesBorins]

Hey all, is there an update on this? should we consider a strategic initiative around it?

[AndreasMadsen]

@JiaLiPassion We now have a public API for catching error the way domain does it. And we have migrated the domain module to use the API.

• docs: https://github.com/nodejs/node/blob/master/doc/api/process.md#processsetuncaughtexceptioncapturecallbackfn
• domain: https://github.com/nodejs/node/blob/master/lib/domain.js#L103

Additionally, there is a general tracking issue for implementing domain using only public APIs nodejs/node#17143

[JiaLiPassion]

@AndreasMadsen , Thank you for the update, I will look into it. And I will also check the never resolved promise issue.

[AndreasMadsen]

@JiaLiPassion we now have the promiseResolve event. so that issue should be possible to solve now too :)

[JiaLiPassion]

@AndreasMadsen , got it, I will check it, thank you!!!

is the new promiseResolve can handle this case?

 const p1 = new Promise(() => {}); // This will never be resolved
 const p2 = new Promise((resolve) => setTimeout(resolve, 1000, p1)); // resolve p2 with p1 after 1s

I will test it.

[AndreasMadsen]

@JiaLiPassion I'm not sure it can. I think it just emits the first resolve value. It would be smart if we could know the "resolved" value.

[JiaLiPassion]

@AndreasMadsen , got it, I will test it.

[jasnell]

Discussion here appears to have stalled. Is there a next step or can we close this issue?

[bmeck]

@jasnell I've never gotten a clear statement on if we want to continue to block the language level feature still. It seems ok to close, but I suspect this issue will continue to be raised at a language level.

[jasnell]

yeah, I doubt this issue is going to be enough to get the existing TSC membership organized enough to form a coherent opinion. If there is forward movement on the zones proposal, we can review at that time, I suppose.

[bmeck]

@jasnell there is no momentum / Zones is stalled due to block from Node on Error handling ( domenic/zones#9 ), the champion sees Error handling as mandatory to move forward.

[jasnell]

Ok, how about sometime in the next few months we queue this back up for a more active TSC discussion?

[jasnell]

@nodejs/tsc ... @mcollina and I have been discussing the issues around Zones and some of the requirements around continuation local storage in general. We've been kicking around a few ideas that I've captured here: https://docs.google.com/presentation/d/1N8w-Ees6sy5qcqh8y5Fc271pcicbI4AB1D8eCIl_1aQ/edit?usp=sharing

I'd like to get some discussion going among ourselves to see if this is an approach we could rally behind before taking it to TC-39.

[fhinkel]

@jasnell was this discussed at the last TC39 meeting?

[ljharb]

@fhinkel it was not discussed in March; the next TC39 meeting after that comment was posted will be at the end of May.

[fhinkel]

@ljharb Thanks!

@jasnell, does this need to be discussed by the TSC or would @nodejs/diagnostics be a better place to handle this?

[jasnell]

Yes. Both :)

[mike-kaufman]

If someone could tag this with diag-agenda, we'll bring it up at next diag WG meeting. Here's my .02, plz let me know if you disagree 😄 :

There are two fundamental problems w/ "async" in javascript:

• Problem 1: Shared semantics & terminology. My read on why the Zones proposal bogged down, is there's no way to talk about & reason about asynchronous execution in JavaScript. There's lots of different strategies that can be applied to how an error propagates across an async call graph, and different approaches have different tradeoffs, but being able to discuss these at the right level of abstraction is critical. We've updated some of the terminology we've been working on here, and would appreciate any feedback.

• Problem 2: Understanding async boundaries in user code. For the javascript programmer, understanding async boundaries is difficult, primarily because there's no syntactic construct in JS code to understand where these occur.

It is our belief (hope?) that if we get the concepts & terminology in 1) correct, then language constructs and VM support should follow easily.

RE Zones, it provides some capabilities over the async call graph, namely continuation-local-storage and error propagation. However, there are other desired capabilities as well, e.g., post-mortem analysis, or a debugger stepping across async boundaries "in context".

Given all of this, I think Zones is a bit premature. If something goes to TC39, I would prefer it focus on first-principles and address those two fundamental problems above. If we can get those things nailed, then we can talk about user APIs to support CLS or error async error propagation or anything else. Moreover, with the right model in place, if someone doesn't like Zones behavior, they're free to implement their own over the primitives.

/cc @bterlson, @mrkmarron

[mhdawson]

This was discussed in the latest Diagnostic WG meeting. At this point we believe we need to close on the work on async formalization before we might look at something like this. Given that and that we have nobody stepping up as a champion going to remove from diagnostic agenda for now.

[Trott]

Closing this due to inactivity and it likely being out-of-date, but feel absolutely free to hit the re-open button if that's not the right thing to do.