Consistent sync and async function names

[jonathanong]

Specifically all synchronous functions should be *Sync and all other functions should be async. Mixed usage would be deprecated, but not removed for backwards compatibility.

For example, we should deprecate crypto functions like crypto.randomBytes(length) in favor of crypto.randomBytesSync(length).

There is no request for additional functionality. PR welcomed?

Reference: nodejs/node-v0.x-archive#7030

[indutny]

Haha, let the discussion begin! :)

[calvinmetcalf]

I believe it's just crypto that still has problems right? Some places where async/non-blocking could help

• createDiffieHellman(length) this can take a very long time so a non blocking version of this would be helpful, as this is a constructor not a method it would require a bit more of an api tweak
• diffieHellman computeSecret, and generateKeys
• sign, verify, publicEncrypt, and privateDecrypt
• async, non-streaming encryption/decreption, authenticated encryption methods are fundamentally incompatible with streaming because you are supposed to authenticate before doing stuff with the data, a encrypt method which took an algorithm, data, password or key/iv and for gcm aad and a tag on decryption could have a sync an async versions (maybe slip in some stronger key derivation).
• non blocking hash stream? not sure if it would be much of a benefit.

[iefserge]

Would be nice to be able to call any async function as sync like this:

var sync = require('sync');
var fs = require('fs');

var stats = sync(fs.stat(file))

This could also work for user defined functions.

[vkurchatkin]

@iefserge related issue nodejs/node-v0.x-archive#7323.

Proof of concept 1: https://github.com/vkurchatkin/deasync
Proof of concept 2: vkurchatkin@5477d44

[nmn]

I think that while Node should be slowly cleaned up and made more consistent, it would be a mistake to change too much too soon in an overeagerness to improve things.

Instead, I think the best way forward would be to embrace the existing API and extend it with new features from ES6. (It's in everyone's best interest if Node.js stays consistent with the standards)

That said, renaming functions to make them more consistent with the general naming scheme is obviously a great idea. But let's not overhaul how sync and async functions work at the moment.

There are various ways to easily wrap packages as Promises or for generators. That is the best approach for now, as even though callbacks can be painful, they are the lowest level code possible, and can be wrapped to be compatible with Promises, Observables, Generators, Thunks and Channels. That flexibility is worth something.

[mikeal]

@iefserge that isn't actually possible. The best we could do would be

var sync = require('sync');
var fs = require('fs');

var stats = sync(fs.stat)(file)

[mikeal]

@Naman34 that is already happening above core, see: co and thunkify for instance.

In general, if something is being accomplished well in the ecosystem then core should stay out of it.

[yoshuawuyts]

@iefserge I'd argue that we probably need less synchronous functions, not more. Having a sync() function is extremely powerful, but ultimately I think it would tilt the platform in the wrong direction (e.g. away from being an async event machine).

Actually I'd be interested in what would happen if all sync functions would be removed entirely. I've had the impression many of the sync functions have been added just for the hell of it. Not sure if possible though, haha, mostly an interesting thought experiment.

[mikeal]

@yoshuawuyts the module system relies on a bunch of sync calls, so they can't be removed entirely.

[bnoordhuis]

createDiffieHellman(length) this can take a very long time so a non blocking version of this would be helpful, as this is a constructor not a method it would require a bit more of an api tweak
diffieHellman computeSecret, and generateKeys
sign, verify, publicEncrypt, and privateDecrypt

Agreed. This seems uncontroversial.

async, non-streaming encryption/decreption, authenticated encryption methods are fundamentally incompatible with streaming because you are supposed to authenticate before doing stuff with the data, a encrypt method which took an algorithm, data, password or key/iv and for gcm aad and a tag on decryption could have a sync an async versions (maybe slip in some stronger key derivation).

I'm not sure what you mean here. That you need to buffer before you can encrypt/decrypt/verify?

[iefserge]

@mikeal

that isn't actually possible

It's possible, but fs.stat needs to return some special async operation id value. And sync(id) blocks on that operation.
But var stats = sync(fs.stat)(file) would work as well.

[vkurchatkin]

fs.stat needs to return some special async operation id value

or simply a promise.

[darrenderidder]

Promises, like the many other async abstractions that exist, belong in user modules.

[nmn]

In general, if something is being accomplished well in the ecosystem then core should stay out of it.
@mikeal You basically said what I was trying to say in a concise way.

@darrenderidder: +1 to what you said.

I think sync function were added mostly for building command-line utilities. Let's not forget that node isn't ONLY for web servers.

[vkurchatkin]

Promises, like the many other async abstractions that exist, belong in user modules.

Yes, and that is why they are a part of the language

[iefserge]

@vkurchatkin Current promises implementation in v8 is very slow, too much overhead to create a promise for every async operation. Simple int id would work.

[nmn]

@vkurchatkin Promise is going to be part of the language soon. But it's not the right approach for all things yet. Promises suck for event handlers, like the callback for http.createServer.

Yes, there are other places where Promises are awesome to work with. But It's nice to have consistency in the API. And wrapping core features in Promises is SO TRIVIAL, it's not even worth messing with the core to save you a few lines of code per project.

Also, I really like what Pete Hunt said,
“Just because something is a standard, doesn't make it ‘the future’”

[vkurchatkin]

@iefserge I honestly don't think that promises can be a bottleneck. Eventually they could be fast and well optimized, so this is a sort of investment.

Your idea requires weak maps (I think), otherwise memory leaks are possible:

var registry = new WeakMap;

function asyncFn () {
   var id = Symbol();
   var operation = { done : false, result : null, error : null };
   registry.set(id, operation);

  someCallbackBasedFn(function (err, res) {
     operation.done = true;
     if (err)
      operation.error = err;
     else
       operation.result = res;
  });

  return id;
}

If we store registry as a Map or simple object we have to keep operation record indefinitely.

[vkurchatkin]

@Naman34 event listeners and cps callbacks are not the same. Listeners should be simple functions, no doubt. http.createServer is really a bad example: what's the point of making something an event if it is supposed to have a single listener? Better API (in my opinion) would be:

  var server = createServer(function (request) {
    return new Promise(function (resolve, reject) {
      resolve(new http.Response({/* some options */}))
    })
  });

[calvinmetcalf]

@bnoordhuis when you decrypt something with GCM, you shouldn't do anything with the decrypted data until you authenticate it, which means in practice you should buffer it on decryption.

[mikeal]

I honestly don't think that promises can be a bottleneck. Eventually they could be fast and well optimized, so this is a sort of investment.

Node doesn't intentionally make things slower. Ever. @trevnorris would literally burn this whole mother down, and with good reason.

There aren't actually any applications in Node that don't use third party modules outside of userland. The fact that promises can be accomplished easily in userland with no performance hit to the rest of core means they'll likely never be added to core itself. That doesn't mean people can't use them and build an ecosystem with them, you're more than welcome to, but it's silly to continue to ask for core to adopt them when there is a measurable penalty for doing so.

[jonathanong]

whoa whoa this issue for me was just about deprecating primarily crypto functions like crypto.randomBytes(length) in favor of crypto.randomBytesSync(length). nothing about different control flow mechanisms or new features.

[mikeal]

@jonathanong maybe you can update the title and description to be more specific and limit the scope :)

[jonathanong]

can we create a iojs/discussions repo for this type of control flow discussions?

[mikeal]

@jonathanong we're talking about core, it should probably be an issue in here.

[bnoordhuis]

this issue for me was just about deprecating primarily crypto functions like crypto.randomBytes(length) in favor of crypto.randomBytesSync(length)

That seems perfectly acceptable to me. I pulled the name out of thin air when I added crypto.randomBytes(), no thought was put into it. In retrospect, it's kind of an odd duck compared to other sync/async core APIs.

[trevnorris]

@mikeal Thanks. I need to write a post about core API and Promises/etc. that we can point to. This is a topic that's continuously brought up.

[trevnorris]

About adding API's like writeSync(), the problem is (as @piscisaureus pointed out to me) if there are asynchronous writes already in the pipeline to be written the sync call would need to pause execution until all async writes are complete. Doing that reliably is nothing trivial.

[jonathanong]

@trevnorris do you object to just making the crypto APIs consistent? if no one objects, i'll try to make a PR this weekend, but first i'll have to figure out how you guys do deprecation messages