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

Feature Request: Every async function returns Promise #11

Closed
rdner opened this issue Nov 29, 2014 · 323 comments
Closed

Feature Request: Every async function returns Promise #11

rdner opened this issue Nov 29, 2014 · 323 comments
Labels
discuss Issues opened for discussions and feedbacks. feature request Issues that request new features to be added to Node.js.

Comments

@rdner
Copy link

rdner commented Nov 29, 2014

Now in node we have callback or EventEmitter model to deal with async calls by default. But in my opinion it is better if every async function returns a native Promise (from new version of V8).
It does not break backward compatibility and supports optional callback if needed.

If so we just do not need to install additional package for promises like q or bluebird except additional functionality is needed.


_EDIT 2014-12-11 by @rvagg_

Comment lifted from here so it's easier to see for newcomers to this conversation.

This was discussed at the TC meeting yesterday, see #144, the aim was to be able to provide at least some kind of statement as feedback in this issue. I don't think the issue needs to be closed and can continue to collect discussion from those who feel strongly about this topic.

The feedback from the TC about incorporating a Promises-based API in core goes something like this:

A Promises API doesn’t make sense for core right now because it's too early in the evolution of V8-based promises and their relationship to other ES* features. There is very little interest within the TC in exploring this in core in the short-term.

However, the TC is open to change as the feature specifications and implementations in ES6 and ES7 are worked out. The TC is open to experimentation and providing the most optimal API for users, which may potentially include a Promises-based API, particularly if newer features of JavaScript work most optimally in conjunction with Promises. The speed of the language specification process and V8 implementation will mostly dictate the timeline.

It should be noted that a callback API is unlikely to ever go away.


@jonathanong
Copy link
Contributor

#5 + time is a prerequisite IMO. otherwise, you can't do this without breaking people's shit.

@rdner
Copy link
Author

rdner commented Nov 29, 2014

What exactly will break "people's shit"? They still be able to work with callbacks, but also will have ability to do something like that:

server
  .listen(3000)
  .then(function() {
    // some action on ready event
   })

or they still can

server
  .listen(3000, function() {
    // some action on ready event
   })

Not only for servers of course, for every async function in node, including modules like fs, net, dns etc.

@jonathanong
Copy link
Contributor

because some crypto functions without a callback actually return the result synchronously, so you can't change it to a promise without breaking API.

@rdner
Copy link
Author

rdner commented Nov 29, 2014

And they will do it without any changes. I said nothing about synchronous functions, only about asynchronous that work with callbacks now.

@TJkrusinski
Copy link

@pragmadash right, so since some of the crypto functions are variadic and operate synchronously in the absence of a callback they couldn't return a promise without breaking API changes.

@darrenderidder
Copy link

-1 Promises were explicitly removed from node years ago in favor of allowing flow control abstractions to be implemented in user modules. This has proven to be a good decision since it allows developers to work with abstractions they find appropriate while keeping core as simple as possible.

@indutny
Copy link
Member

indutny commented Nov 29, 2014

I'm not really a big user of user facing APIs, but when I do use them - I'd prefer callbacks over the promises. They don't have any overhead, and promises could be implemented on top of them. So -1 for promises.

@Qard
Copy link
Member

Qard commented Nov 29, 2014

There's also the issue that native promises in V8 are still kind of broken.

Speaking purely as someone that writes instrumentation for node, promises would be nice because I could just check if the return type of a call is a promise and tack on instrumentation via the then(...) function rather than manually searching the arguments for a callback which is error-prone.

In the long run, I think generators are a much better way to deal with asynchrony though. Even without any JIT optimization currently, they are substantially faster than promises. They aren't as fast as callbacks though. Callbacks work fine, and are currently the most efficient option.

@vkurchatkin
Copy link
Contributor

There's also the issue that native promises in V8 are still kind of broken.

In what way?

@medikoo
Copy link

medikoo commented Nov 29, 2014

-1 While I'm fan of promises, I wouldn't like to see node.js API converted, @indutny put it well

@Qard
Copy link
Member

Qard commented Nov 29, 2014

@vkurchatkin

Unhandled rejection is still not a solved problem. Errors that occur in the context of a promise and are not explicitly handled in the usage of the promise do not propagate upward to anywhere that they can be handled: try/catch around the new Promise(...) doesn't catch them, domains don't catch them, etc.

https://gist.github.com/Qard/4758942da01a9b7dd6e1

@vkurchatkin
Copy link
Contributor

@Qard it doesn't mean promises are broken. Everything works as defined by the spec

@medikoo
Copy link

medikoo commented Nov 29, 2014

@vkurchatkin exactly, they're broken by spec ;-)

@Qard
Copy link
Member

Qard commented Nov 29, 2014

@medikoo Agreed. Silent failure is very dangerous for production apps.

There was some recent attempt at adding a function to Isolate that could be given a callback to catch unhandled rejections. Not sure what the status of that is though. Until it's available in V8 and handled in node, I wouldn't consider native Promises a safe thing to use.

@defunctzombie
Copy link
Contributor

This issue should be locked. It will be bikeshed to no end. ES as a language is still exploring the "async" space and how that will be handled. For now we should stick with core language features and less library features. Promises is a library feature. Functions are a language feature right now. In the future it may become clearer that what the idiomatic approach is.

@jtmarmon
Copy link

Imo this should just be revisited when ECMAScript 6 comes out with native promises

edit: wow, totally didn't know promises are native in v8. thanks for pointing that out @Qard

@Qard
Copy link
Member

Qard commented Nov 30, 2014

Native promises are already in V8. The issue is not native support, but a series of minor things like lack of error safety, and poor performance compared to callbacks. (Though bluebird has proven comparable native performance should be possible eventually)

@rvagg
Copy link
Member

rvagg commented Nov 30, 2014

@Qard have you had a good look at the benchmark suite that bluebird uses to "prove" its claims? I'd take any results coming out of that with a grain of salt. I'm happy to admit they've made impressive progress with a heavy abstraction--but it's still a heavy abstraction that's more suitable for userland than core.

@Qard
Copy link
Member

Qard commented Nov 30, 2014

@rvagg Yep. I'm aware it's still a bit heavier than raw callbacks. It has improved substantially from initial implementations though, and from the state of the native implementation even, but callbacks are still better.

Perhaps someday promises will have comparable performance--particularly in combination with arrow functions, since they don't have the overhead of needing to create a new context. For now, callbacks are simply the best option and it's yet to be seen that any other will ever be better.

@rlidwka
Copy link
Contributor

rlidwka commented Nov 30, 2014

This issue should be locked. It will be bikeshed to no end. For now we should stick with core language features and less library features. Promises is a library feature.

I agree with that. It's easy enough to wrap node.js stuff in promises as is.

@spion
Copy link

spion commented Dec 1, 2014

I use and love promises, advocate them, am satisfied with the very low overhead of modern promise libraries and I think that Bluebird nails the error handling issue with its reporting of possibly unhandled errors.

That said, I still think that Promises don't belong in node core. I really like the libuv.js direction of exposing an even lower level API to userland and letting users decide on the high level abstractions that are best suited to their project.

The only thing I would like are more consistent lower-level APIs that are easier to e.g. automatically promisify on demand as well as richer Error types (more error codes and/or flags). Another thing that would really boost performance and lower memory use would be the ability to pass context objects to the api exposed by libuv.js which would give better optimization opportunities to all userland control flow libraries (not just promises)

@YurySolovyov
Copy link

-1. Core should be as lightweight as possible.
Bluebird actually has a great feature called Promisification

@ntkoso
Copy link

ntkoso commented Dec 1, 2014

Currently i'm using 'channels' instead of promises. Other users are using co with thunks. And even with promises there are multiple choices what library to use. Rewriting 'every async function' in core to 'new ES7\8\9+ standard way of handling async operations' every time the spec changes is bad idea.

@KoryNunn
Copy link

KoryNunn commented Dec 2, 2014

-1.

it is extremely trivial to convert CPS to Promises and there are libs to do this.

My hammer doesn't need Bluetooth.

@MauriceButler
Copy link
Contributor

As @indutny said, I prefer callbacks over the promises. If you however prefer promises then that is fine, you can add your promise implementation of choice on top of that.

-1 for promises

@SomeoneWeird
Copy link
Member

-1 not in core

@rdner rdner closed this as completed Dec 2, 2014
@hax
Copy link
Contributor

hax commented Dec 3, 2014

+1.

Promise is already in ES6 spec and most new W3C specs are based on Promise. Even Co has been refactored to promise based.

I believe there will be no other new async model in future ES7 -- note the aysnc/await proposol is also promise based.

@darrenderidder
Copy link

Promise whack-a-mole commence!

@benjamingr
Copy link
Member

-1 promises are awesome, but converting the base class library to promises is already a:

var promisified = Promise.promisifyAll(require("yourModuleName")); 

Which works flawlessly with bluebird promises. That said - fixing stuff like fs.exists would be nice since it doesn't obey the convention.

@phpnode
Copy link

phpnode commented May 13, 2015

https://www.npmjs.com/package/q

242,947 downloads in the last day
1,246,495 downloads in the last week
5,011,270 downloads in the last month

https://www.npmjs.com/package/promise

64,074 downloads in the last day
335,511 downloads in the last week
1,188,324 downloads in the last month

https://www.npmjs.com/package/when

31,687 downloads in the last day
171,934 downloads in the last week
563,284 downloads in the last month

https://www.npmjs.com/package/kew

47,818 downloads in the last day
245,334 downloads in the last week
991,012 downloads in the last month

https://www.npmjs.com/package/bluebird

141,484 downloads in the last day
720,362 downloads in the last week
2,851,684 downloads in the last month

Total: 10,605,574

@rlidwka By this flawed metric, promises are just as popular as callbacks

@Fishrock123
Copy link
Member

We've already got promises, I'm less convinced right now we should pull in a second Promise implementation.

I understand that the native impl isn't the greatest right now, but it should be on the road to improving and hopefully it does.

@rlidwka
Copy link
Contributor

rlidwka commented May 13, 2015

By this flawed metric, promises are just as popular as callbacks

@phpnode , by this flawed metric, promises are just as popular as a single control flow library based on callbacks. Which means callbacks are guaranteed to be more popular than promises (because hey, you can use callbacks without a library, but can't use promises without a library (es6 excluded 'cause b/w compatibility still requires one)).

I'm more interested to know how many big projects are internally callbacks/promises based. And I haven't seen anything promise-based in a while.

@pesho
Copy link
Contributor

pesho commented May 13, 2015

I'm less convinced right now we should pull in a second Promise implementation.

I don't think anyone is (seriously) suggesting that.

Some of us mention Bluebird only to show that excellent Promise performance is actually possible (because the bad performance of the "natives" is sometimes used as an argument against Promises in general). And according to @domenic there is good chance that native V8 promises will reach or exceed Bluebird's performance level in the future.

@benjamingr
Copy link
Member

We've already got promises, I'm less convinced right now we should pull in a second Promise implementation.

No one is suggesting that - just to be clear. I was just making the point promises are widely used.

@waynebloss

The only thing I ask for is no breaking changes

No one is suggesting this, don't worry.

@ChALkeR
Copy link
Member

ChALkeR commented May 13, 2015

you can use callbacks without a library, but can't use promises without a library

@rlidwka, I'm using Promises without a library.

@Qard
Copy link
Member

Qard commented May 13, 2015

Comparing download counts is a really unreliable metric, considering how npm doesn't pull extra copies of a package when a matching version already exists further up the tree. I have apps where 20-30 different dependencies use bluebird, but they all reference the single top-level copy I have, so 20-30 potential downloads becomes 1 actual download.

Regardless of which is more popular, the language itself is developing in a particular way, and I don't think we should go against the grain of that by refusing to support promises ourselves. New users will have certain expectations when coming from developing in the browser, and will just be confused when they find those assumptions to be wrong in the server.

At the same time, I definitely agree that we can't break the existing ecosystem though.

@benjamingr
Copy link
Member

The fact promise libraries have a lot of downloads is a reliable metric to show that there are at least some parts of the community that use promises - not to claim they are the most popular thing ever :)

It was just countering:

just because some group decided that Promises should be "native" in ECMA, it doesn't mean people will actually use them.

Nothing more, nothing less.

@CrabDude
Copy link

This will all be a moot point if async/await reaches mainstream adoption. Errbacks will seem primitive and obsolete with no upside in core beyond legacy support, causing core APIs to appear likewise to node developers, JavaScript developers, and as @benjamingr points out the rest of the software community who will be using more stable and mature abstractions.

Is there any rational justification for the use of errbacks beyond legacy support? Every justification for their removal from core has been addressed: no-spec, non-peformant, complex control-flow. Not to mention that they predated async/await and enterprise (large scale stable production) node.

Errbacks are:

  1. More complex and difficult to comprehend than async/await
  2. Offer expectations with no guarantees: no catch, single value, single resolution, asynchronicity, etc...
  3. Don't support asynchronous stack traces
  4. Not composable
  5. No faster
  6. As prone to silent failure due to manual error passing
  7. Supported nowhere else in the community
  8. Openly ridiculed outside the community

So, we can keep revisiting this, making the same arguments that will carry more and more weight each time, or we can establish hypothetically what support looks like and the conditions for its inclusion, put together a PR, refine it till it meets all expectations, and accept it once that conditions have been met.

@trevnorris
Copy link
Contributor

I'm going to have to call BS on your performance claims. As it currently stands normal callbacks are far more performant. This may change in the future as V8 improves implementation of these new features, but is definitely not true today.

@CrabDude
Copy link

I'm going to have to call BS on taking my performance claim out of context. Promise libraries are currently as fast, and as mentioned everywhere here, capable of native performance parity in the future.

@trevnorris
Copy link
Contributor

Is there any rational justification for the use of errbacks beyond legacy support? Every justification for their removal from core has been addressed: no-spec, non-peformant, complex control-flow.

That seems to very directly say that using err style callbacks is less performant than using new APIs. Which is currently not true. Don't think I took that out of context.

@CrabDude
Copy link

This will all be a moot point if async/await reaches mainstream adoption.

Errbacks will seem

the conditions for its inclusion [...] once that conditions have been met.

Those seem to very directly say that the initial, continued and final context are a future in which mainstream adoption has occurred and the expectation of performance parity is met. (See 3rd Edit)

Edit: Also, the purpose in even talking about a "hypothetical" future is stated at the begging of the sentence:

So, we can keep revisiting this, making the same arguments that will carry more and more weight each time, or we can establish hypothetically what support looks like and the conditions for its inclusion

2nd Edit: @trevnorris This stands to be proven whether a performant promise implementation in core is in fact necessarily slower. Any unoptimized version will nearly always be slower than an optimized version. So let's compare apples with apples. Let's add this as a prerequisite for accepting this issue. Are there any other conditions, or is this the only one?

3rd Edit: When I say that the issue of promises being non-performant has been met, I mean that it is now shown to be possible to return promises and be as as performant as passing a callback. To prove it, one would need of course to implement and benchmark it for comparison.

@bmeck
Copy link
Member

bmeck commented May 14, 2015

Lets just start with not talking about potential futures of VM optimizations unless we get a go ahead from a VM implementor that it will happen. Don't flood my inbox ppl. Promises are not optimized as of right now, their potential should not guide action if that potential is not proven.

@arcanis
Copy link
Contributor

arcanis commented May 14, 2015

And their performances right now should not guide action either, since they are now a language construct.

The issue is not really an issue of performances...

@bmeck
Copy link
Member

bmeck commented May 14, 2015

@arcanis it is definitely a memory and cpu hit to move to them, anything that causes those in core should be very carefully regarded.

@arcanis
Copy link
Contributor

arcanis commented May 14, 2015

From my point of view, Io.js is, more than just a Node fork, a way to playtest the ES6 features (it's more complex than that, of course, but you get the gist).

ES6 promises are not yet another hype: they are in the language, they will stay, and are now the standard way for a js program to notify that a task has been completed. Of course, their performances are not yet on par with regular callbacks, how could they be? The engine implementations are still fresh, and cannot yet compare with something whose every possible optimization has been tried (I guess :).

Anyway, I'm still talking about perfs but my point is unrelated to them: promises being standard, it seems to me that we have nothing to debate except "ok, how to get them in the standard Node library without breaking BC?". The discussion "should we include them in the langage? are they a good enough solution?" has already been done by the ecmascript commitee. Now, it's up to the library authors to use them and give feedback.

As a side note, look at the C++ standard library. Despite C++ being almost exclusively used for its perfs, the standard library is focused on being portable and relatively easy to understand. The perfs come only third. That's imo how a standard library should be designed : it should closely follow the language constructs, so that a beginner can start hacking without much second thought. Then, once the need arise, switch to a specialized library, focused on perfs.

By keeping the callback API as the only Io.js API, we would only force every Io.js developer to live with a constant premature optimization.

@Fishrock123
Copy link
Member

From my point of view, Io.js is, more than just a Node fork, a way to playtest the ES6 features (it's more complex than that, of course, but you get the gist).

In reality, not really.

What we can do in io.js (or in the converged node project) is to put things like this behind flags for testing, if it can be done reasonably. (Like the workers impl)

@benjamingr
Copy link
Member

@arcanis

As a side note, look at the C++ standard library. Despite C++ being almost exclusively used for its perfs, the standard library is focused on being portable and relatively easy to understand. The perfs come only third.

The whole point of C++ is that it's a zero cost abstraction and you don't pay a performance penalty for features you don't use. This statement is in complete contrast to everything I've ever read about C++ and/or lectures I've heard.

Check out "B. Stroustrup: The Design and Evolution of C++. Addison Wesley, ISBN 0-201-54330-3.
March 1994": What you don’t use, you don’t pay for (zero-overhead rule).

By keeping the callback API as the only Io.js API, we would only force every Io.js developer to live with a constant premature optimization.

No one is suggesting that, promises are already decided - we're just waiting for them to be ready for prime time.

@tracker1
Copy link

Honestly, I don't think we should remove any of the existing callback implementations, too much existing structure relies and/or builds on them. That said, Promises are here to stay, and will become the defacto way to build stuff... Moving forward, async/await will expand this much, much farther.

import u from '../../../utility';
export {processOne as default}

async function processOne() {
  let [q,table,index] = await Promise.all([
    u.getQueue('stowListing'),
    u.getTable('listing'),
    u.getEsIndex('listing')
  ]);
  var msg = await q.one();
  if (!(msg && msg.value)) return null; //no record in place

  var {accountId,listingId} = msg.value;
  var data = (await table.read(accountId,listingId)).dataObject;
  await index.upsert(listingId,data);
  await q.done(msg);
  return listingId;
}

I won't even begin to describe what that workflow would look like without Promises. This is code I am using today using BabelJS... my sincere hope is that I'll be able to do this natively in iojs/node within a year, without having to transpile.

Using callback patterns directly would be a lot more work, a lot more verbose, and a lot more prone to error. Regardless of the performance issues... code that works, is better than code that is broken and/or more prone to errors. For now, there's mz, which wraps internal libraries in promisified versions... for that matter, I setup bluebird as my global promise implementation because it tends to work better, and have more features than the native version... all of that said, striving for this in core is a good idea.

@greim
Copy link

greim commented May 14, 2015

Hasn't the window of opportunity long since closed on using an import switch, given how much transpiler-supported ES6 code already exists in the wild? It would lock people into their transpilers forever.

@vkurchatkin
Copy link
Contributor

@greim people who want to migrate from transpiling would have to replace import of builitns with require. it's not that bad, actually. Or we can publish a module that simple re exports old APIs, so that people could simple change import fs from 'fs' to import fs from 'old-node/fs'

@CrabDude
Copy link

@greim No, not really. It's really only an issue for published packages, and publishing packages with import. For application developers, it's as simple as replacing import with require in application code, but that window will likely close in 6 months.

@greim
Copy link

greim commented May 15, 2015

I'd still argue for callback detection for a few reasons:

  • The behavior change is triggered by changing how you call the API in question, rather than how you use an unrelated language feature.
  • Popping the errback off the argument list moves the action to the return position, which has a certain symmetrical feel to it.
  • Errbacks aren't going away, so some sort of quirk will be left floating around in node as a result of this; there's no way around it. The nature of the quirk might as well itself be descriptive of the reason for its existence.
  • It can be done with negligible impact on performance.
  • It leaves the user free to choose.
  • While it would almost certainly break someone's code somewhere (every major node version does), it should be extremely rare, and wouldn't otherwise lock large swaths of folks out of upgrade paths pending code rewrites. Node is already evolving so fast that avoiding schisms (ala python 2/3) should be on everyones' worry list. We don't know which hair will break that camel's back.

Anyhow, there's my 2¢, carry on.

@Temptin
Copy link

Temptin commented May 30, 2015

I agree with dual functionality; either returning promises or using callbacks depending on how the functions are called. The only problem is how freakishly slow V8's "native" promises are. They are written in JavaScript and aren't even close to Bluebird's performance. So until the performance issue with promises is resolved, I don't see any value in changing node to support them. They're just too slow right now and would be a hindrance to Node's performance.

Would be interesting if someone with contacts at Google could find out if they're going to improve their promises implementation. If not, then I suggest avoiding promises like the plague.

@ChALkeR
Copy link
Member

ChALkeR commented Jun 5, 2015

Was the fact that v8 Promise implementation is slow ever raised on the v8 issue tracker? I can't find a corresponding issue, either open or closed.

I assume that's one of the blockers of promisifying core.

And one other thing that I don't get: why are v8 promises implemented with mixed native/js code, when pure js implementations are faster?

@benjamingr
Copy link
Member

@ChALkeR they are very well aware of the issues - see Domenic's comments here

@Fishrock123
Copy link
Member

If people are interested, discussion should be moved to the NG repo: https://github.com/nodejs/NG/issues

Closing & locking this because it's mostly bikesheding that no-one has the time to read though. @rvagg's comment in the OP still stands.

@nodejs nodejs locked and limited conversation to collaborators Jun 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discuss Issues opened for discussions and feedbacks. feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

No branches or pull requests