Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add HostPromiseRejectionTracker #76

Closed
wants to merge 1 commit into from
Closed

Conversation

domenic
Copy link
Member

@domenic domenic commented Oct 2, 2015

This is a non-observable change that will allow HTML to provide events for tracking promise rejections, as outlined in https://github.com/domenic/unhandled-rejections-browser-spec.

On top of #75 for convenience.

@benjamingr
Copy link
Contributor

👏👏 Great to see this finally progress.

domenic added a commit to whatwg/html that referenced this pull request Oct 2, 2015
This new feature uses a pair of events on global scopes, viz. unhandledrejection and rejectionhandled, to allow authors to track unhandled promise rejections. The specification was originally drafted at https://github.com/domenic/unhandled-rejections-browser-spec in collaboration with @jeisinger and @bzbarsky.

This feature depends on tc39/ecma262#76.
@bterlson
Copy link
Member

bterlson commented Oct 6, 2015

I am bought off on the necessity of some affordance for Promise rejection tracking for dev tools. What I don't yet understand are the following:

  1. What are the compelling scenarios for making this a user-facing API?
  2. What are the security constraints in doing so? I imagine there are similar constraints to onerror where the origin of scripts throwing the error have to be considered. Possible to enumerate these for those of us less familiar with HTML-land (me :-D).
  3. Why is this being done in a host-dependent way? Would it be better to pursue a solution that fits entirely in 262 so all JS environments can benefit from this machinery?

These aside, this could be contentious so I think we should discuss this in November at the F2F before I pull it in. I want be careful not to compromise our consensus process by overzealously merging PRs.

@domenic
Copy link
Member Author

domenic commented Oct 6, 2015

Sure, happy to help!

What are the compelling scenarios for making this a user-facing API?

This was outlined in the original proposal, which maybe I should have included in the commit message. Essentially, the use case is the same as for window.onerror: telemetry.

What are the security constraints in doing so? I imagine there are similar constraints to onerror where the origin of scripts throwing the error have to be considered. Possible to enumerate these for those of us less familiar with HTML-land (me :-D).

Yeah, exactly; it's origin-related. Actually, thank you for the reminder; I had not taken care of that properly in whatwg/html#224 yet. But yes, you should not be able to read error messages or stack traces from cross-origin scripts. This is taken care of by the "muted errors" flag, which you can click to see all the places it impacts.

Why is this being done in a host-dependent way? Would it be better to pursue a solution that fits entirely in 262 so all JS environments can benefit from this machinery?

Error reporting mechanisms vary greatly across hosts. Web browsers have multiple global scope types, each with their own type of event loop and resulting way of enqueuing tasks to notify as such. There you use onunhandledrejection and addEventListener("unhandledrejection", ...) in conformance with existing platform conventions, and to allow easy adaptation by libraries that are already using window.onerror. In contrast, Node.js has two separate "microtask-esque" queues, their nextTick queue and the V8 microtask queue, which interact with the timing of this event. And it uses the process.on("unhandledRejection", ...) mechanism, which is conceptually similar to browser EventTargets but different in many specifics, up to and including the conventional casing of the event name used.

Although in theory providing such machinery in 262 would be nice, in that it would allow writing portable code for this sort of thing, in practice doing so would be both difficult and have a low payoff. It would be difficult in that it would require creating new machinery for this type of event handling or live list or similar, and driving that to consensus. And it would require specifying the event loop at a much deeper level than ES currently does; as you can see from e.g. https://github.com/whatwg/html/pull/224/files or https://www.w3.org/Bugs/Public/show_bug.cgi?id=25981, the level of detail needed for a feature like this is pretty great, and that's not even trying to be compatible with environments outside browsers. Finally, it would not be that useful either, since error-reporting mechanisms are not really something you want portable code for. Reporting client-side errors in a browser-based web app is going to be done very differently than logging server errors in a Node.js server, or logging runtime errors in an embedded device. All of the telemetry uses are going to require some kind of I/O, so the code isn't going to be all that portable anyway.

These aside, this could be contentious so I think we should discuss this in November at the F2F before I pull it in. I want be careful not to compromise our consensus process by overzealously merging PRs.

Seems quite reasonable to me. Feel free to tag this with "pending f2f" or something. I'll put the topic on the agenda.

domenic added a commit to tc39/agendas that referenced this pull request Oct 6, 2015
@domenic domenic force-pushed the unhandled-rejection-hooks branch 2 times, most recently from c1cf104 to 018cd75 Compare October 6, 2015 18:40
@bterlson bterlson added needs consensus This needs committee consensus before it can be eligible to be merged. editorial change labels Oct 6, 2015
@domenic domenic force-pushed the unhandled-rejection-hooks branch 5 times, most recently from b3b0826 to 35036ac Compare November 17, 2015 18:12
@bterlson
Copy link
Member

Per Nov 18 TC39 meeting, this is good to go. Additionally, TC39 has empowered the editor to allow changes to improve the layering of ECMAScript with other relevant specifications when the semantics are unobservable. Only requirement is that they are called out explicitly in the release notes. 🎆

@benjamingr
Copy link
Contributor

👏🏻 awesome

domenic added a commit to whatwg/html that referenced this pull request Nov 20, 2015
This new feature uses a pair of events on global scopes, viz. unhandledrejection and rejectionhandled, to allow authors to track unhandled promise rejections. The specification was originally drafted at https://github.com/domenic/unhandled-rejections-browser-spec in collaboration with @jeisinger and @bzbarsky.

This feature depends on tc39/ecma262#76.
@bterlson
Copy link
Member

@domenic can you rebase?

@anba
Copy link
Contributor

anba commented Nov 21, 2015

[[PromiseIsHandled]] needs to be added to the list of internal slots in 25.4.3.1 Promise, step 3.

@domenic
Copy link
Member Author

domenic commented Nov 23, 2015

Rebased and fixed @anba's find. Not sure what prefix to use besides "Normative" or "Editorial" since this is kind of a third category... will leave that to @bterlson.

This is a non-observable change that will allow HTML to provide events for tracking promise rejections, as outlined in https://github.com/domenic/unhandled-rejections-browser-spec.

For more information on the original proposal and use cases (mostly on the HTML side), see https://lists.w3.org/Archives/Public/public-whatwg-archive/2014Sep/0024.html.

The corresponding HTML pull request is at whatwg/html#224.
domenic added a commit to whatwg/html that referenced this pull request Nov 25, 2015
This new feature uses a pair of events on global scopes, viz. unhandledrejection and rejectionhandled, to allow authors to track unhandled promise rejections. The specification was originally drafted at https://github.com/domenic/unhandled-rejections-browser-spec in collaboration with @jeisinger and @bzbarsky.

This feature depends on tc39/ecma262#76.
domenic added a commit to whatwg/html that referenced this pull request Nov 30, 2015
This new feature uses a pair of events on global scopes, viz. unhandledrejection and rejectionhandled, to allow authors to track unhandled promise rejections. The specification was originally drafted at https://github.com/domenic/unhandled-rejections-browser-spec in collaboration with @jeisinger and @bzbarsky.

This feature depends on tc39/ecma262#76.
@bterlson
Copy link
Member

Committed as e89e31d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change needs consensus This needs committee consensus before it can be eligible to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants