Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

async/await and Node #7

Closed
benjamingr opened this issue Jun 9, 2016 · 37 comments
Closed

async/await and Node #7

benjamingr opened this issue Jun 9, 2016 · 37 comments

Comments

@benjamingr
Copy link
Member

benjamingr commented Jun 9, 2016

Async/await has landed in V8, details are being worked out. This is a feature many users have been waiting for and I suspect many will use.

We need to figure out how to work with it in core, namely:

  • We need to finally make up our mind about what happens when promises throw and there is no catch handler attached in a reasonable time span.
  • We need to figure out how core would work with async functions would work in core and with core functions.
  • The CTC need to make a decision about Adding Core support for Promises node#5020 and consider how (and if) it would work with async functions.
  • Node core might want to be involved in proposals regarding typed catch and other proposals that are needed for async/await to work in Node.
  • We need to solve the promises related post-mortem issues. Creating a promise no longer requires using the word "Promise" anywhere - all you have to do is (async () => {})(). Post-mortem core-dump users would have to run a transform on their code forbidding async and await otherwise.

I would also love to see async/await in the context of Node discussed in the next TC meeting if possible. Our window of action is rapidly closing and if there is a grave mistake we need to find it asap.

@benjamingr benjamingr changed the title Async/await and Node async/await and Node Jun 9, 2016
@i5ting
Copy link

i5ting commented Jun 11, 2016

nice job

@benjamingr
Copy link
Member Author

Going to get some people talking here since this requires attention and I think a lot of collaborators are unaware, cc @nodejs/collaborators

@bmeck
Copy link
Member

bmeck commented Jun 14, 2016

Note: TC39 has been discussing, but has not formally proposed "top level await" for Modules.

@yorkie
Copy link

yorkie commented Jun 14, 2016

I'm -1 on adding a ES feature which is landed at v8 but not as a finished proposal. BTW, do we need to consider the https://github.com/nodejs/node-chakracore as well?

@bmeck
Copy link
Member

bmeck commented Jun 14, 2016

@yorkie async functions are proposed in https://github.com/tc39/proposals and have advanced pretty far, top level await is await outside of async functions.

@yorkie
Copy link

yorkie commented Jun 14, 2016

@bmeck I mean the finished proposal, which has no items about async function, and the async function feature is currently in the stage 3.

Let me know if I'm wrong :-)

@bmeck
Copy link
Member

bmeck commented Jun 14, 2016

@yorkie Stage 3 means it is going to land, pending implementation problems / concerns for environments. That means it is the appropriate time for Node to get involved and the last chance to voice concerns before it lands in ECMA262.

@ChALkeR
Copy link
Member

ChALkeR commented Jun 14, 2016

@yorkie I would prefer us not moderating language features that landed in v8 at our end. Also, most likely the proposal will be accepted (stage-4) before async functions will be enabled in v8 by default (i.e. without a flag).

@benjamingr
Copy link
Member Author

@yorkie if Node waits until the ink is dry then it has no chance to bring issues up - that has been a big pain point before.

Async await already landed in ChakraCore a while ago.

We need to make sure we're ready and need to expect at least some widescale adoption as this feature has seen in C#.

@vkurchatkin
Copy link

@benjamingr I don't think that async-await actually requires special treatment, so the fact that it's in v8 shouldn't change anything for core

We need to finally make up our mind about what happens when promises throw and there is no catch handler attached in a reasonable time span.

If anything, this is less important for async-await, because everything has rejection handler, except top level promise.

We need to figure out how core would work with async functions would work in core and with core functions

Nothing to decide, async functions are just functions that return promises.

Node core might want to be involved in proposals regarding typed catch and other proposals that are needed for async/await to work in Node.

Why is it needed for async-await? Try-catch is not something new.

We need to solve the promises related post-mortem issues

Also not something new. I seriously doubt that someone actually monkey-patches promises to have post-mortem support.

Our window of action is rapidly closing

Seems like a huge overstatement. What worst case scenario you have in mind?

@mikeal
Copy link

mikeal commented Jun 14, 2016

I don't think it sets a good precedent to disable features V8 has landed. We have traditionally looked to V8 to define its own stability level and take it "as is." This opens up a big can of worms and potentially endless bike shedding and second guessing of V8 which I don't think we want to get into.

@trevnorris
Copy link

We can't "disable" this feature. Because it's not an API. It's syntax. We can not support it by not returning promises.

@mikeal
Copy link

mikeal commented Jun 15, 2016

We can not support it by not returning promises.

People wrap our APIs in promises, including the native ones ship with. With await landing we can only expect that to increase. The burden of supporting these use cases in core, whether we return promises in core APIs or not, will increase with the increase in usage in the ecosystem and there isn't much we can do about it.

@jasnell
Copy link
Member

jasnell commented Jun 15, 2016

Debating whether or not we want to support a syntax level feature is not
going to be worthwhile. Yes, we need to improve the promises story in core
but they're here to stay and so is async/await. At this point the
discussion needs to be how do we best support it or how do we limit
negative impact.
On Jun 15, 2016 8:28 AM, "Mikeal Rogers" [email protected] wrote:

We can not support it by not returning promises.

People wrap our APIs in promises, including the native ones ship with.
With await landing we can only expect that to increase. The burden of
supporting these use cases in core, whether we return promises in core APIs
or not, will increase with the increase in usage in the ecosystem and there
isn't much we can do about it.


You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
#7 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAa2eUlGM_gg3d7SVZUSEJuJmc99TaVwks5qMBorgaJpZM4IyN9w
.

@Fishrock123
Copy link

If you modify global.Promise does that change what async functions return? If not we are in for a lot of performance hurt...


Imo the steps that would need to be taken just come around to better native promise support:

  1. Get V8 to expose the microtask queue. If they don't, we need to move to floating a patch onto V8 for ourselves to make the stupid thing actually usable.
  2. Begin investigating existing APIs that are hard/impossible to wrap with promises well and try to move towards interfaces that can be wrapped easier.

As for what we do about unhanded rejects... idk, this makes them sync like and there will be more behavior dissonance. I think we should bite the bullet and make unhandled rejects crash (by default..) in v7. Maybe we could have a way of denoting specific promises that disobey that, unsure.

Perhaps we begin wrapping all modules in an async function? That way we can add a top-level catch handler and maybe make the job a bit easier on ourselves? Idk I just came up with that now so probably not.

@ChALkeR
Copy link
Member

ChALkeR commented Jun 17, 2016

Perhaps we begin wrapping all modules in an async function? That way we can add a top-level catch handler and maybe make the job a bit easier on ourselves? Idk I just came up with that now so probably not.

That will magically make await work in scripts and modules even outside of async functions. That doesn't look like a good thing to do.

@chrisdickinson
Copy link

@Fishrock123:

If you modify global.Promise does that change what async functions return? If not we are in for a lot of performance hurt...

IIUC, changing what global.Promise points at does not modify the result of await immediateValue — it becomes a native Promise for immediateValue no matter what global.Promise is pointed at.

Perhaps we begin wrapping all modules in an async function? That way we can add a top-level catch handler and maybe make the job a bit easier on ourselves? Idk I just came up with that now so probably not.

Unfortunately we'd have to be able to spin the microtask queue forward in order for this to work — the result of an async function is always a Promise, and Promise's will always resolve, at minimum, "on next tick" (on next microtask run.)

@benjamingr
Copy link
Member Author

async/await is now live in Chrome. If there is any action we want to take with regards to it - now is the time.

@rvagg
Copy link
Member

rvagg commented Jun 30, 2016

panic

Is this an acceptable response?

@eljefedelrodeodeljefe
Copy link

Yes.

@Fishrock123
Copy link

Wait doesn't it require ES modules for the syntax...?

@benjamingr
Copy link
Member Author

benjamingr commented Jun 30, 2016

@Fishrock123 no, what you are thinking of is top-level await. You can use async/await for functions regardless of modules.

@ronkorving
Copy link

@benjamingr Source? And which version of Chrome?

@benjamingr
Copy link
Member Author

@ronkorving

Chrome 53 Canary - windows 64 bit (without any opt-in experimental flags or custom options) has them, Chrome 51 doesn't yet.

The platform status says it requires a flag but in practice it does not.

@ilkkao
Copy link

ilkkao commented Jun 30, 2016

async/await is behind --harmony_async_await in Chrome 52. v8 5.1 just got merged to master. I'm secretly hoping that @targos or somebody else pushes initial v8 5.2 branch to somewhere soon :) That'd
allow me to start experimenting.

@Fishrock123
Copy link

If you override the Promise global object, does async/await use that instead?

@bmeck
Copy link
Member

bmeck commented Jun 30, 2016

@Fishrock123 it uses the intrinsic %Promise%, cannot be changed via JS : https://tc39.github.io/ecmascript-asyncawait/#abstract-ops-async-function-await

@benjamingr
Copy link
Member Author

@Fishrock123 there was a proposal that allowed overriding what await does by @jhusain ~ a year ago but it was not well received by the TC - https://github.com/jhusain/compositional-functions .

This puts a lot of burden on what the platform must provide in terms of hooks and instrumentation for native promises and it means we'll have to deal with what to do on unhandled rejections better - I'm finally going to be able to work on my PR for warning next weekend which is a first step but not a solution.

@trevnorris
Copy link

@mikeal

The burden of supporting these use cases in core, whether we return promises in core APIs or not, will increase with the increase in usage in the ecosystem and there isn't much we can do about it.

Must have not been clear enough that I responded in regards to your comment of "I don't think it sets a good precedent to disable features V8 has landed." Specifically at your use of the word "disable". I wasn't giving my stance on the topic.

We need to solve the promises related post-mortem issues.

Can we determine whether this is actually solvable? Since Promises must be asynchronous, and they don't differentiate between calling rejected() or throwing an exception, what mechanics exist that would allow for aborting on exceptions?

Are we going to question what impact this will have on the ecosystem, in terms of existing Promise implementations? Meaning, will this be the beginning of the end for libraries like bluebird?

@targos
Copy link
Member

targos commented Jul 1, 2016

@ilkkao WIP here: https://github.com/targos/node/tree/v8-5.2

@ilkkao
Copy link

ilkkao commented Jul 1, 2016

@targos I just converted my hobby project from Babel to native async/await and used your branch. Everything seems to just work. Thanks!

@chicoxyzzy
Copy link

chicoxyzzy commented Jul 15, 2016

async/await is behind --harmony_async_await in Chrome 52

This doesn't work in my Canary 54 on macOS 10.12

It does

@xjamundx
Copy link

xjamundx commented Aug 4, 2016

@yorkie you probably saw async/await is stage-4 now and does exist in this list now:
https://github.com/tc39/proposals/blob/master/finished-proposals.md

@targos tried out your branch, that's pretty cool that it works with the --harmony_async_await flag!

@benjamingr
Copy link
Member Author

I'm wondering if we're better off just providing a util.promisify function.

@ronkorving
Copy link

@benjamingr that might be a great solution. I can imagine patterns like this in the top of my files:

const promisify = require('util').promisify;
const readFileSync = promisify(require('fs').readFileSync);

Not super ideal, but it's a nice utility function. Inevitably, I think people will release promisified modules (eg: fs-promises), but they too could use this utility function.

Alternatively:

const fs = require('fs').promisified;

Could be nice, because the APIs could be exposed and stay uptodate with changes in the fs module (in this case). Just a thought, or is this too much too soon?

@CrabDude
Copy link

CrabDude commented Aug 29, 2016

@benjamingr @ronkorving

Your example demonstrate a drawback of a util.promisify approach in that it's a regression from the current shift from require to import (from dependency loading via runtime execution toward dependency loading via declaration (hoisted, parse-level)), and brings with it the associated potential for mistakes.

Specifically, promisifying *Sync is a non-sensical anti-pattern. By returning a promise on a *Sync function, you have now created a contradictory function that both suggests it's non-blocking (it's not) and blocking (which it is regardless of the variable name).

[Remainder of comment moved to #12]

@benjamingr
Copy link
Member Author

Closing due to lack of discussion. Feel free to reopen.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests