Specifying (or not) sync/async resolution behavior #4

Closed
briancavalier opened this Issue Oct 24, 2012 · 48 comments

Comments

Projects
None yet
Owner

briancavalier commented Oct 24, 2012

From #1

Without getting into which is "better", I think the more important initial question is:

Does such a requirement actually belong in this proposal?

There are quality implementations of Promises/A in the wild that do each, and in my experience, they interoperate without problems. For example, I have mixed when.js, Dojo Deferred, jQuery Deferred, and Q in real, deployed applications over the past 2 years, and have never had a problem that I could attribute to one of them not doing async resolutions.

So, I think we need to answer the question of whether this belongs in the spec or not before we can discuss the obvious follow-on question :)

Member

domenic commented Oct 24, 2012

Interoperation is not the issue. We don't need a spec to interoperate, since we can assimilate any thenable, even jQuery ones.

Owner

briancavalier commented Oct 24, 2012

Ok, right. Then, why would we include anything at all about this in Promises/A+? What value does it provide to the proposal given that useful implementations of each approach exist?

(Honestly not trying to sound condescending with those questions, I really do want to hear the answers. Like I said over on the cujojs group, I'm open to this, but I need to see the value)

Member

domenic commented Oct 24, 2012

(No worries.)

The value is the same as every other part of Promises/A+. "Useful" implementations exist that don't follow various parts, and we can always just assimilate such implementations. If that level of interoperability is the only goal, Promises/A+ is entirely unnecessary. But we want a standard so people using promises and creating future promise implementations all support the same basic features, like composition, error bubbling, or async resolution.

Note I'm not (yet) trying to convince you that async resolution is important. I'm just saying that the argument that implementations exist without it and interoperate fine is not a good one. I'll work on the former later; trying to dig up some Mark Miller papers.

Owner

briancavalier commented Oct 24, 2012

My perspective is that since useful implementations of each do exist, and have been in use for some time, I wonder why this is a necessary requirement. Most of those implementations converged on the same behavior for handler-returned promises. That makes it enough of a defacto standard (at least in my mind), to codify the behavior in Promises/A+.

However, they haven't converged on async vs. sync. Now, one might say that they haven't converged because it's not specified in Promises/A, but one might also say that it simply isn't a necessary requirement. IOW, I wonder why things have (mostly) converged on handler-returned promises, but not sync vs. async.

Definitely looking forward to reading what you dig up!

wycats commented Oct 24, 2012

Repeating my comment from #1


There are quality implementations of Promises/A in the wild that do each, and in my experience, they interoperate without problems. For example, I have mixed when.js, Dojo Deferred, jQuery Deferred, and Q in real, deployed applications over the past 2 years, and have never had a problem that I could attribute to one of them not doing async resolutions.

Guaranteeing async frees the developer to use desirable patterns, such as binding the handler and then doing other setup that will be used in the handler.

It is of course always possible to modify the code to do the setup before binding the resolution handler, but it's quite nice to know that you don't have to. Certain styles of code really benefit from this capability.

On the flip side, allowing synchronous delivery opens the door to common kinds of bugs. In particular, promises that are usually delivered synchronously but may occasionally be delivered asynchronously allow a style of programming that depends on the synchronous delivery but will fail in the face of (rare) asynchronous delivery.

var value;

promise.then(function(val) {
  value = val;
});

// do something with value

You can say "that's a bad idea", but people program "until it works", and if synchronous delivery dominates a particular promise resolution, you could easily imagine a lot of code like this in the wild that would fail strangely in a few cases.

Owner

briancavalier commented Oct 24, 2012

Thanks @wycats. The question at hand is whether the proposal should include a requirement that dictates a delivery mode, or whether the proposal should allow the promise implementor to decide, as Promises/A did.

So, putting aside the fact that you feel async is better than sync, do you also feel that the proposal should include a requirement on the delivery mode?

I think your point about programming "until it works" may be a good one, especially as promises become used more widely by less experienced Javascript developers. And at least one of the goals of Promises/A+ in my mind, is to help promises be used more widely.

As usual, though, I'll provide a counterpoint: when.js currently does synchronous resolution, and has a reasonably substantial user community. I can honestly say we've never had a single issue filed where someone was tripped up by sync delivery. Developers seem to be getting along fine with promise implementations that do both, which makes me wonder why we should lock it down.

Member

domenic commented Oct 25, 2012

Here is Mark Miller's reasoning. It's rather theoretical, but strong. I definitely agree with his "In addition, programs will often be tested only in one of these two cases, leading to unexpected bugs when the other case occurs in the wild." And looking at the table he references is worth doing, just to drive home how bad the situation is (again, in theory).

I guess this doesn't really address your desire for a counterexample where one gets tripped up by synchronous delivery. Unfortunately I'm not in much of a position to deliver one, since I've never programmed with synchronous delivery, but perhaps I can trawl through my codebase and look for places where we're making the assumption.

Contributor

lsmith commented Oct 25, 2012

I can't help but see this discussion slippery sloping to defining progress handler behavior. Cherry picking what to flesh out from Promises/A will produce another incomplete spec.

+1 to people programming "until it works".

I do feel that enforcing consistency in implementation to a spec benefits everyone. Easier to move from implementation to implementation (changing libs), and more likely to be adopted as a proposal to ES. The goal is to get rid of libraries. This requires the spec to be explicit. That addresses my take on whether it should be in the spec.

My opinion on specific behavior is that it be asynchronous, though I haven't resolved whether each of many handlers should be invoked in their own thread, or all synchronously from the same thread.

Member

unscriptable commented Oct 25, 2012

Thanks @wycats. Your code sample and "until it works" explanation opened my eyes (ninja edit):

var value;

promise.then(function(obj) {
  value = obj;
});

value.doSomething(); // FAIL!

The fact that a noob mistake like this will fail early is a key advantage to adoption, imho.

Ok, cool. Now I'm leaning towards async behavior as the default, but allowing sync as an option for advanced users. There will always be situations when sync behavior is desired. For instance, curl.js (and probably wire.js?) performs an order of magnitude faster than other AMD loaders while in "dev mode" (using un-compiled modules) in old IE because it resolves modules sync instead of async. Maybe that's an outlying case, but I think most devs would appreciate the speed and wouldn't mind setting some config option to get it. :)

Member

domenic commented Oct 25, 2012

My opinion on specific behavior is that it be asynchronous, though I haven't resolved whether each of many handlers should be invoked in their own thread, or all synchronously from the same thread.

I assume by "thread" you mean "turn of the event loop".

I can't see any argument for separate turns of the event loop. All the traditional next-turn mechanisms (e.g. process.nextTick, setImmediate, or setTimeout(, 0)) will execute separately-queued functions in the same turn.

Although, when polyfilling, you need to be careful that your manual queue preserves the inability of separately-queued functions to interfere with each other (e.g. if function 2 in the queue throws an exception, function 3 still needs to get executed). See tildeio/rsvp.js#20.

Contributor

lsmith commented Oct 25, 2012

I assume by "thread" you mean "turn of the event loop".

Yep, less typing, and I knew everyone knew what I meant. I can use the more verbose description for semantic correctness if that's a pet peeve of yours ;)

All the traditional next-turn mechanisms (e.g. process.nextTick, setImmediate, or setTimeout(, 0)) will execute separately-queued functions in the same turn.

Wha? Are you saying setImmediate(fnA); setImmediate(fnB) would execute both in the next tick, allowing an error in fnA to prevent the execution of fnB?

Although, when polyfilling, you need to be careful that your manual queue preserves the inability of separately-queued functions to interfere with each other.

If synchronous in the same turn of the event loop, then each handler would need to be wrapped in a try/catch to protect against the issue you raised on rsvp.js. That leaves open the question of what to do with those errors. I don't think anyone likes the idea of ignoring errors.

But the runtime cost of each handler given its own turn of the event loop could be noticeable, though I'd suggest in edge cases. In practice, I think of promises as granular transactions, which suggests one or few handlers is the vast majority case. If that's true, the event loop queuing cost would be measured from the initial queuing of either the first handler or all handlers. That would make the time distinction between "all handlers in next tick with trapped errors" vs "each handler in its own tick" largely moot. The error trapping is a real concern, though, as it affects application behavior (rather than timing) and makes bug discovery harder.

Member

domenic commented Oct 25, 2012

Wha? Are you saying setImmediate(fnA); setImmediate(fnB) would execute both in the next tick

Yes

, allowing an error in fnA to prevent the execution of fnB?

No: just like setTimeout(, 0) or process.nextTick, they do not interfere with each other. This naturally occurs in browsers with postMessage, MessageChannel, onreadystatechange, and friends. Indeed, I believe it would work with MutationObservers if you don't try to get clever and manage the queue yourself, instead just letting the browser do it.

As to what happens to the errors, they get thrown, and are uncatchable (just like setTimeout(, 0)).


I think there's some confusion (possibly caused by myself) on trapping errors in fulfilled and reject handlers, versus trapping them in general asynchronously-executed functions. You generally want to asynchronously execute something like this:

process.nextTick(function () {
    var newValue, newReason;
    try {
        newValue = fulfilled(prevValue);
    } catch (e) {
        newReason = e;
    }

    if (newValue) { resolveNewPromise(newValue); }
    if (newReason) { rejectNewPromise(newReason); }
});

So you never lose errors from promise handlers. I was talking about the more general issue of how asynchronous behavior works.

wycats commented Oct 25, 2012

I assume by "thread" you mean "turn of the event loop".

Yep, less typing, and I knew everyone knew what I meant. I can use the
more verbose description for semantic correctness if that's a pet peeve of
yours ;)

I actually wasn't immediately sure what you meant by "thread", since that
term means something different outside the context of JavaScript.

All the traditional next-turn mechanisms (e.g. process.nextTick,
setImmediate, or setTimeout(, 0)) will execute separately-queued functions
in the same turn.

Wha? Are you saying setImmediate(fnA); setImmediate(fnB) would execute
both in the next tick, allowing an error in fnA to prevent the execution
of fnB?

Since the goal is mostly to eliminate a footgun, I don't see any additional
benefit by forcing additional turns beyond the first. I should note that
setImmediate executes after paint (unlike most implementations of
postMessage and Mutation Observers), which is important if the callbacks
are affecting the DOM.

Although, when polyfilling, you need to be careful that your manual
queue preserves the inability of separately-queued functions to interfere
with each other.

If synchronous in the same turn of the event loop, then each handler would
need to be wrapped in a try/catch to protect against the issue you raised
on rsvp.js. That leaves open the question of what to do with those errors.
I don't think anyone likes the idea of ignoring errors.

Presumably the same as if an error happened inside the callback? The
problem of swallowing errors is a general problem that afflicts
well-implemented promises implementations, and I will propose a solution
(previously discussed with @domenic) in another issue.

But the runtime cost of each handler given its own turn of the event loop
could be noticeable, though I'd suggest in edge cases. In practice, I
think of promises as granular transactions, which suggests one or few
handlers is the vast majority case. If that's true, the event loop queuing
cost would be measured from the initial queuing of either the first handler
or all handlers. That would make the time distinction between "all
handlers in next tick with trapped errors" vs "each handler in its own
tick" largely moot. The error trapping is a real concern, though, as it
affects application behavior (rather than timing) and makes bug discovery
harder.

The performance is most acutely an issue with older browsers that need to
use setTimeout, but there are cases where forcing new turns of the event
loop would be costly, and again, I am unsure what the benefit is.

Contributor

lsmith commented Oct 25, 2012

No: just like setTimeout(, 0) or process.nextTick, they do not interfere with each other.

Ok, my misunderstanding of js engine implementation.

I think there's some confusion (possibly caused by myself) on trapping errors in fulfilled and reject handlers...

I was confused, but think I'm with you now. The invocation of the notification process should be in the next tick, but each handler will necessarily be try/catch wrapped to enable those handlers' corresponding promise fulfillment or rejection. As each is individually wrapped, they needn't be individually scheduled with setImmediate or its ilk since they can't interfere with each other. Right?

@wycats I'll keep an eye out for your new issue, but I think my previous comment is just a confused rant, and safe to ignore.

Member

domenic commented Oct 25, 2012

The invocation of the notification process should be in the next tick, but each handler will necessarily be try/catch wrapped to enable those handlers' corresponding promise fulfillment or rejection. As each is individually wrapped, they needn't be individually scheduled with setImmediate or its ilk since they can't interfere with each other. Right?

Yup, I think we're all on the same page now :). Honestly I do think I confused matters by not clearly separating the general issue of scheduling from the specific issue of promise handlers; see my update to the RSVP bug.

Owner

briancavalier commented Oct 25, 2012

Great discussion, guys. I'm coming around to believing we should put async in the spec :)

FWIW, I like the simplicity and style of the language in the UncommonJS Thenables spec on this subject:

Callbacks should not be called before then returns.

Maybe it's too loose tho, given that it took a few detailed posts here to really understand the specifics? Thoughts on how much guidance should we provide?

Member

domenic commented Oct 25, 2012

Maybe it's too loose tho, given that it took a few detailed posts here to really understand the specifics? Thoughts on how much guidance should we provide?

Hmm. I think the UncommonJS language suffices, and is indeed elegantly sufficient, because given that JavaScript is run-to-completion, there's no possible way for us to execute the callbacks synchronously but also before then returns.

I might lean towards something that's a bit easier to translate directly into tests, but no strong feelings.

Owner

briancavalier commented Oct 25, 2012

How does this sound?

"fulfilled and broken must not be called before then returns"

Member

domenic commented Oct 25, 2012

Works for me!

Contributor

lsmith commented Oct 25, 2012

+1

awwx commented Nov 15, 2012

Urmph... I realize that this has already been decided (closed), but I really dislike the async requirement.

The strength of the Promise abstraction is that callbacks can be attached before or after the promise is resolved. Clean, simple, elegant.

The async requirement to my mind makes an compliant implementation unnecessarily heavyweight. I, personally, would not want to use a library which implemented Promises/A+ with the async requirement.

Should an implementation be free to make callbacks async? Of course. Even I would enjoy using a "debug mode" which made callbacks async in my program, to help me catch my dumb errors.

I'm delighted that domenic pointed me at the setImmediate implementation library (I had no idea that it existed, and I'm very happy that I now know about it), but to think that I'd have to use it for promise callbacks? Ug.

The async requirement strikes me as conflating two different features (promises and "run callbacks in another tick"). Features can be combined in different ways. For example, one way to combine the features would be to have a global flag ("make all callbacks async in this Promise implementation"). But, I haven't tried to implement this so I don't know if it's possible (not sure if I'm thinking inside-out), but suppose one could write a utility function that would wrap or replace a then call and which would ensure that these callbacks were async. Then as a user I would have the choices of making all callbacks async, or none, or particular ones but not others.

Member

domenic commented Nov 15, 2012

@awwx just checking, but did you get a chance to read the thread top to bottom? There's been quite a lot of discussion about this, which managed to convince a few folks that were on your side originally.

awwx commented Nov 15, 2012

mmmm.

I have to say, my first reaction was that I really didn't like it, but I'm having trouble so far coming up with a convincing argument...

One can of course write buggy code that works asynchronously but breaks synchronously:

var value = "old";

promise.then(function () {
  value = "new";
});

// proceed with the expectation that value currently is still "old".
assert.equals(value, "old");

but, in practice, probably the novice programmer is not going to make that mistake because they'll be reading the code linearly top to bottom.

The async requirement may be less efficient... but that true for high-level languages in general trading off increased usability for some decrease in efficiency.

I notice that IndexedDB has an interesting feature of auto-inactivating unused transactions:

If you make a transaction and return to the event loop without using it then the transaction will become inactive. The only way to keep the transaction active is to make a request on it. When the request is finished you'll get a DOM event and, assuming that the request succeeded, you'll have another opportunity to extend the transaction during that callback. If you return to the event loop without extending the transaction then it will become inactive, and so on. As long as there are pending requests the transaction remains active.

Sadly this means we couldn't use promises for IndexedDB, which is too bad because a promise interface would be much nicer than the onsuccess/onerror DOM events. But simply relaxing the async requirement wouldn't help, because the callbacks would have to be required to be synchronous for it to work...

medikoo commented Jun 10, 2013

The problem with forced async resolution, is that it discourages re-use of promise objects whenever they resolve, as then any value access is forced to resolve at next tick. It's similar problem to one we have, when working with not well designed event-emitter, which internally forces emit of events to next-tick, totally undesirable.

There are already issues related to that in issues tracker of popular promise implementations, and it looks the real fix is to provide counterpart sync interface on a side. On contrary I never seen a related issues in a promise library that doesn't force a next tick resolution.

The problem here described by @wycats is a school error, which very easily can be workaround when spotted (and honestly, it doesn't happen when you already know your way in promises). However problems that come out of forced next tick resolution are much worse and cannot be workaround that easily. I think we should really re-think that case.

Please, please, please reverse this decision! Forced async resolution causes me frequent pain and is awful!

I use promises for almost everything. I find them to be a great abstraction. Often for the sake of consistency and expressive power I use promises even for code that I know will run synchronously. In some cases I even have code that must run synchronously where I would like to be able to use promises.

Consider an example that I happen to be looking at right now implemented with when.js. I'm working with Bacon.js and I wanted to write a helper function to grab the current value of a property. Grabbing a value requires using a callback; wrapping that up in a promise makes perfect sense:

function getCurrent(property) {
    return when(function(resolve) {
        property.onValue(function(v) {
            resolve(v);
            return Bacon.noMore;
        });
    });
}

I know that the onValue callback will execute synchronously because a Bacon property (almost) always has a current value. But when.js implements Promises/A+, so it turns my helper into an asynchronous function. It happens that I want to grab a property value in the middle of a section of bookkeeping code and making that asynchronous leaves an opening for my application to get into an inconsistent state.

That puts me in a position where I have to either fork and modify when.js, switch to an implementation of Promises/A that allows sync resolution, or ditch promises and use lame callbacks for cases like this.

In other words, there is a class of use cases that Promises/A+ cannot accommodate. I want to be able to use promises for everything - but I can't.

It has been pointed out repeatedly in this discussion that there are some cases where forcing async resolution can avoid bugs. But I don't think that justifies the cost. You can make certain classes of bugs less likely; but doing so limits the cases in which promises can be used.

Besides, async resolution opens up its own class of potential bugs in the form of race conditions. For example, here is some code to open a database connection:

var dbConnection = when.reject();

function connect(url) {
    return dbConnection
    .then(function(conn) {
        return conn.open ? conn : when.reject();
    })
    .then(noop, function() {
        dbConnection = asyncConnectionInit(url);
        return dbConnection;
    });
}

The idea is that there should only be one connection. If connect is called multiple times the application should not establish multiple connections while the first connection attempt is still initializing.

I wrote this example to take full advantage of promise abstractions. For consistency dbConnection is initialized with a failed promise so that it can be assumed that variable is never undefined. The check for the open state in the connection returns another failed promise if the connection has closed so that I only had to write the initialization routine once.

If promise resolutions ran synchronously then this code would work nicely. But if resolutions are async then connect could be called a second time before the fail handler assigns a new promise to dbConnection. That causes a second connection attempt to be made, thus breaking the desired contract.

To produce code that can handle async resolutions I have to add some extra convolutions:

var dbConnection;

function connect(url) {
    if (!dbConnection || !dbConnection.conn.open) {
        initConnection();
        return dbConnection;
    }
    else {
        return dbConnection.then(noop, function() {
            initConnection();
            return dbConnection;
        });
    }

    function initConnection() {
        dbConnection = asyncConnectionInit(url);
        dbConnection.then(function(conn) {
            dbConnection.conn = conn;
        });
    }
}

Note the undefined check and the hackish assignment of a synchronously-accessible value to the resolved promise.

Race conditions tend to be subtle. I believe that they will be more difficult to identify than code that unexpectedly runs synchronously.

One of the major advantages of promises is that they allow the developer to explicitly express dependencies between pieces of code. Because of this code that may run asynchronously will work just as well, if written properly, if it runs synchronously instead. In my opinion the only advantage of forced async resolutions is to accommodate, ahem, shoddy code. I do not want my abstractions watered down to cater to people who do not understand those abstractions.

Finally I want to point out, as others already have, that it is really easy to turn a synchronous callback into an asynchronous one - but the reverse is not possible. If promise resolutions ran synchronously then developers would have the option of determining whether any given promise resolves synchronously or asynchronously. But with forced async resolutions developers have no such option. This forces application code to adapt to the library instead of the library being adaptable to different kinds of code.

Contributor

juandopazo commented Aug 17, 2013

I'm pretty sure you can fix all your use cases without changing how promises work (and the result will probably be easier to understand).

In your first example the key part is "because a Bacon property (almost) always has a current value" where "almost" is crucial. Promises work for values that may or may not be available at the time. If you know your code will run synchronously, you don't need a promise, you can just write:

function getCurrent(property) {
    var value;

    property.onValue(function (v) {
      value = v;
    });

   return value;
}

But you're using a promise which means there are cases in which it won't be called synchronously and if you assume it will it's almost certain your application will get into an inconsistent state.

In your second case you're caching something. Instead of caching the result you should be caching the promise. That also makes starting with a rejected promise unnecessary. Here's how it could work assuming you only want one connection per url:

var connect = (function () {
  var cache = {};

  return function connect(url) {
    if (!cache[url]) {
      // assuming asyncConnectionInit() returns a promise
      cache[url] = asyncConnectionInit(url);
    }

    return cache[url];
  };
}());

That's a normal caching strategy for promises: you cache the promise.

In your first example the key part is "because a Bacon property (almost) always has a current value" where
"almost" is crucial. Promises work for values that may or may not be available at the time. If you know your code
will run synchronously, you don't need a promise, you can just write:

By using a promise I can handle the general case where onValue may run asynchronously. But in my application where I ensure that my property always has a value I know that it will run synchronously. It's the best of both worlds. Otherwise there would be a need for two helper functions: one to handle that sync case (which might fail), and one to handle the async case.

In your second case you're caching something. Instead of caching the result you should be caching the
promise. That also makes starting with a rejected promise unnecessary.

I would like you to read my example more carefully. I do cache the promise. But I'm considering a case where there is more involved. My example:

  1. checks whether the connection has closed since the last call to connect
  2. checks whether the last connection attempt failed and if so, makes another attempt

By the way, it is not a lack of familiarity with the territory that is causing me to encounter problems. I have been writing promise-based code for several years as part of my full-time job. I wrote my own promise implementation in 2010, before Promises/A existed. In that implementation I used forced async resolutions, for the same reasons that have been cited in Promises/A+ discussions. But as I gained experience I found that feature to be more trouble than it was worth. Later jQuery introduced its promise implementation and I switched over to using that. When I did I found sync-capable promises to be more generally useful than the strictly async ones. And I do not recall encountering problems, in my code or in code written by my coworkers, as a result of synchronous resolutions.

Contributor

juandopazo commented Aug 17, 2013

I'm possibly not understanding yout use cases, yes.

Otherwise there would be a need for two helper functions: one to handle that sync case (which might fail), and one to handle the async case.

If I understood correctly, that's why we have functions like Q() in Q that transform values into promises. The function that consumes the value uses Q(value) to ensure it's dealing with asynchronous code.

But I'm considering a case where there is more involved.

The reason I insist that refactoring is the way to go is because we know that treating promises as values leads to code that ends up being pretty similar to synchronous code, but once promise callbacks start having side-effects things get unnecessarily complicated. We also know that the reason it seems that using side-effects is a logical way of using promises is because of the lack of syntax for promises. If you work in a generators+promises environment and write pseudo-synchronous code you'll see you never run into these issues.

While I agree that it's possible 80% of the time there isn't much chance of running into issues with synchronous resolution, there are other things consider:

  • Assuming all promises are asynchronous may be cumbersome at times, but it can't break your code. Assuming some promises are synchronous can definitely break your code when they aren't.
  • Standards are based on consensus. Consensus is generally hard to reach so once it was reached it's even harder to change it. In order for them to change the reasons will probably have to be breaking programs in the wild

tl;dr: There is a specification detail with two possible outcomes. Changing the spec will result in bugs in some programs. Keeping the spec uchanged makes some use cases impossible and will result in bugs in some programs.

If I understood correctly, that's why we have functions like Q() in Q that transform values into promises. The function that consumes the value uses Q(value) to ensure it's dealing with asynchronous code.

The Q() function is not helpful in the cases that I described. But just to be sure that I am following - my guess is that your thinking is that I could use Q() to handle sync and async resolutions of a Bacon property with one function like this:

// Function that retrieves value synchronously.
function getCurrent(property) {
    var value;

    property.onValue(function (v) {
        value = v;
        return Bacon.noMore;
    });

    return value;
}

// Synchronous use of this function.
var value = getCurrent(property);

// Now it looks asynchronous - but it is not.
Q(getCurrent(property)).then(function(value) {
    /* ... */
});

The two use cases were:

  1. Retrieving the current value of a property when it is known that onValue will execute synchronously.
  2. Retrieving the current value of a property when onValue may execute asynchronously.

The use of Q() above will not make use case 2 work since the input expression, getCurrent(property), is a synchronous function. If you try to use Q() in this way in a situation where onValue executes asynchronously what will happen is that getCurrent(property) will return undefined and Q() will turn that into a promise that resolves to undefined.

Nor can I use Q() to build a function that handles asynchronous execution of onValue but that is capable of resolving synchronously if onValue executes synchronously. Once a value gets into an async-only promise, it is async-only forever.

The reason I insist that refactoring is the way to go is because we know that treating promises as values leads to code that ends up being pretty similar to synchronous code, but once promise callbacks start having side-effects things get unnecessarily complicated.

I agree that, if performance issues are put aside, the problem with code executing asynchronously is complications from side-effects. Applications have side-effects and applications have state. It is inescapable. Even when using purely functional patterns wherever possible, if you want to do more than warm up your CPU there must be some side-effects.

In the scenario that I tried to describe above there is an application that may have multiple execution contexts requesting a database connection, where that connection may close unexpectedly requiring that it be reopened, and where by design there should only be one connection at a time. That requires some state management.

I took another shot at an async-safe version of connect after getting some sleep. It seems it can be implemented more cleanly if I can get away with not checking the conn.open status synchronously. That kind of constraint might or might not be so easy in a real application depending on what kinds of factors must be coordinated.

var dbConnection = Q.reject();

function connect(url) {
    dbConnection = dbConnection
    .then(function(conn) {
        return conn.open ? conn : Q.reject();
    })
    .then(noop, function() {
        return asyncConnectionInit(url);
    });
    return dbConnection;
}

The key is to track the last connection lookup in the shared variable on every call to connect.

The point that I wanted to make is that I have to realize that my first implementation has race condition problems and adapt it accordingly. Thus while forcing async resolutions avoids some potential bugs, it exposes other potential bugs.

I'm not really happy with the performance implications either. Making code async can turn code that might have run on the order of tens of microseconds into code that runs on an order of hundreds of microseconds:

$ node
> var start = process.hrtime(); process.nextTick(function() { var end = process.hrtime(); console.log('elapsed microseconds', (end[1] - start[1]) / 1000); });
undefined
> elapsed microseconds 263.664
> var start = process.hrtime(); (function() { var end = process.hrtime(); console.log('elapsed microseconds', (end[1] - start[1]) / 1000); }());
elapsed microseconds 57.916
undefined

We also know that the reason it seems that using side-effects is a logical way of using promises is because of the lack of syntax for promises. If you work in a generators+promises environment and write pseudo-synchronous code you'll see you never run into these issues.

I'm pretty certain that syntax is not my problem. But let's consider the same example written using generators. Here is my first implementation, which is not async-safe:

var dbConnection = Q.reject();

var connect = Q.async(function*(url) {
    try {
        let conn = yield dbConnection;
        return conn.open ? conn : Q.reject();
    } catch(_) {
        dbConnection = asyncConnectionInit(url);
        return dbConnection;
    }
});

And here is the async-safe version:

var dbConnection = Q.reject();

function connect(url) {
    dbConnection = Q.async(function*() {
        try {
            let conn = yield dbConnection;
            return conn.open ? conn : Q.reject();
        } catch(_) {
            return asyncConnectionInit(url);
        }
    })();
    return dbConnection;
}

I have not attempted to write generator code before; so please forgive me if these functions are not correct or are not idiomatic.

It seems to me that I have the same issue either way. And the implementations look about the same to me either way - except that in the promise-only version I have a nice monad and in the generator version I have an awful exception-throwing thing.

I want to point out that anything that can be done with generators+promises can be done with promises alone. The implementation will just look a bit different. If the Promises/A+ specification has genuine problems without generators, then it will have exactly the same problems with generators.

As a primarily browser-side developer I will not be able to use generators for years to come anyway - unless my coworkers warm up to the idea of using a transpiler. To this day I cannot rely even on ECMAScript 5 features like property descriptors due to a requirement to support IE.

While I agree that it's possible 80% of the time there isn't much chance of running into issues with synchronous resolution, there are other things consider:

In my opinion race conditions are also a problem. I do not know whether race conditions will arise more or less often than problems due to synchronous resolution. I do think that problems with synchronous resolution will be easier to identify.

Assuming all promises are asynchronous may be cumbersome at times, but it can't break your code. Assuming some promises are synchronous can definitely break your code when they aren't.

If you make an assumption that is true then of course you will not be wrong. However if I assume that resolutions are always asynchronous that does prevent me from doing things that would otherwise be possible. Or if I don't know that resolutions are always asynchronous, or if I forget to take that into account, then I can end up with broken code.

Standards are based on consensus. Consensus is generally hard to reach so once it was reached it's even harder to change it.

I understand that consensus is hard. I would not be arguing if I did not consider this issue to be important.

There is a point when a specification moves beyond the committee stage and is tested in the real world. From what I understand Promises/A+ has made this step pretty recently. At that point problems always come up that the committee did not anticipate. If the problems are sufficiently serious, the specification must either adapt or die. My purpose here is to give you feedback on the problems that I have encountered in real-world use.

I cannot say definitively that the problems that I describe apply to a broad swathe of real-world use. But I think that they do. Nor can I predict that those problems are sufficiently severe to kill Promises/A+. But I can say with certainty that forced async resolutions limit the usefulness of the specification.

In order for them to change the reasons will probably have to be breaking programs in the wild.

My program is breaking. Does that count?

bergus commented Aug 22, 2013

The decision that then does not call back synchronously was made for consistency. A Promise represents a value that may already be present but also may arrive asynchronously. Since you don't know before (if you did you wouldn't use promises), you must handle the case that it's asynchronous anyway. To simplify things, then is built so that you don't have to handle the synchronous case any different. To make the execution order unambiguous, this spec even allows you to trust in the fact that callbacks will not be invoked before then returns.
That way, it also avoids typical newbie mistakes, reduces unpredictable behaviour, and prevents people from trying to try to synchronously inspect the value (which is hardly ever needed, and for those cases promise libs have special methods).

Your database connection code is a tricky one. However, it actually is a counterexample to your case - it doesn't help if promise resolution was synchronous.

If promise resolutions ran synchronously then this code would work nicely. But if resolutions are async then connect could be called a second time before the fail handler assigns a new promise to dbConnection.

Nope. You seem to fear that connect is called another time between Q.reject(); and the error handler. In an intelligent promise implementation they even might be called directly after each other in the same tick, though you of course cannot trust in that. Yet, the situation does not change if a synchronous execution would be guaranteed: Your actual issue is the asyncConnectionInit, which is always asynchronous. Think of this situation for example:

  1. An initial call to connect
  2. When the fail handler is invoked (irregardless whether synchronous or not), a new promise is created for the connection while that is being established.
  3. After the assignment, another call to connect happens and sets up success and error handler on the dbConnection promise (which is still pending)
  4. And another call to connect does the same
  5. The asyncConnectionInit promise is rejected and triggers its handlers.
  6. Oops. There are two fail handlers now that both try to [re]connect.

To be honest, your program does break because it is buggy not because promises are not synchronous.
Admittedly, your case is a very tricky one. It's not the trivial (common) cache-a-promise (and retry until it gets fullfilled) use case, and your interrupted connection does not break a promise but only can be detected by inspecting the value. Instead, your promise value is becoming invalid at a time and you need to build a new one, so that your connect function actually emits a stream of different promises. Your latest "safe" version which reassigns to dbConnection every time depicts that quite well.
Here is another version that reassigns only in the fail case, but uses promise identity instead of a .conn.open flag. The emitted promises will always be fulfilled with a valid connection, never getting rejected.

var url,
    dbConnection = when.reject();
function connect() {
    var myConnection = dbConnection;
    return dbConnection.then(function(conn) {
        return conn.open ? conn : when.reject();
    }).then(null, function() {
        if (dbConnection == myConnection)
            dbConnection = asyncConnectionInit(url);
        return connect();
    });
}

That is a good implementation. I'll come back to this if I come up with a more compelling use case.

medikoo commented Aug 24, 2013

@bergus consistency argument is flawed. Firstly imagine having tens of resolved promises, and the only way to get to their values is to wait until next tick, this doesn't speak well of promise design.

Consistency argument origins from asynchronous functions, which we expect to resolve earliest at next tick. It's very valid argument in that context, as by calling asynchronous function you always initialize some asynchronous operation. However when you call then or done you do not initialize any new asynchronous process, it makes a big difference. then and done are methods that you may call hundreds times on same promise object, just to get the value that was resolved with one and already initialized asynchronous operation, you must not treat each access as another async operation, it kills performance, it's bad design.

The rule should be:

Any asynchronous function that returns promise, must always return promise that resolves earliest at next tick.

That's it, and we should not force next-tick resolution on then and done.

That newbie error you mentioned is very easy to workaround once caught, and it's unlikely to happen when you know how promises work. I work a lot with promise implementation that doesn't force next-tick on then and never got into that issue.

On contrary you cannot workaround forced next-tick resolution, this restriction actually raises much bigger issues (e.g. cujojs/when#135)

bergus commented Aug 25, 2013

@medikoo:

imagine having tens of resolved promises, and the only way to get to their values is to wait until next tick, this doesn't speak well of promise design.

No. Nothing in the spec says there must not be a way to get their values. It only says that you cannot get them synchronously with the then method.

However when you call then or done you do not initialize any new asynchronous process, it makes a big difference. then and done are methods that you may call hundreds times on same promise object, just to get the value that was resolved with one and already initialized asynchronous operation, you must not treat each access as another async operation, it kills performance, it's bad design.

That's a different thing. We only enforce that the callback is not called synchronously from within the then invocation. When hundreds of handlers are attached to the same promise object, they are allowed to be resolved all in the same tick together - no new async operation is required. Depending on how the promise was constructed, that might even be the same tick as original asynchronous process' resolution. #140 will restrict that last mentioned possibility, but that is a completely different issue than what I was talking about.

medikoo commented Aug 26, 2013

@bergus it may sound like but I didn't suggest that it works like that. I just pointed that artificial asynchronicity is harmful.

When using promises we need to choose between two:

  1. Artificial asynchronicity
  2. Not consistent then behavior

I found that 1 raises really bad issues that can't be workaround and makes promise objects not reusable, while 2 is not really a problem in practice, in 90% of cases you know whether you deal with resolved or unresolved promise. Additionally we're talking about method which behavior is dependent on it's object state, it's not a function that behaves differently in unclear way.

lroal commented Jun 9, 2014

forcing sync operations to wait for next tick is a big design flaw in the spec.

Contributor

petkaantonov commented Jun 9, 2014

Use synchronous inspection

lroal commented Jun 9, 2014

I am gonna implement my own promise library unless there are any existing that does not wait for next tick on sync operations.

Contributor

petkaantonov commented Jun 9, 2014

Example of synchronous inspection http://jsfiddle.net/ukCCA/1/

lroal commented Jun 9, 2014

Thanks

medikoo commented Jun 9, 2014

@lroal deferred doesn't add any artificial asynchronicity

lroal commented Jun 9, 2014

Thanks, @medikoo

pygy referenced this issue in MithrilJS/mithril.js Jul 22, 2014

Merged

nearly Promises/A+ compliant via Promiz.mithril.js #169

maksimr commented Jan 8, 2015

Not clear to me the pros of always async promises.

Case when newbie may write something like:

var value = "old";

promise.then(function () {
  value = "new";
});

console.log(value);

is really rare and easy detected case.

Problems with async behavior appear when you try writing sync tests:

var value = "old";

// I want resolve promise immediately
promise.then(function () {
  value = "new";
});

assert.equal(value, 'new');

With current implementation it's very awkward. I should mock setTimeout function. Or write async tests with their disadvantages.

Thanks

@maksimr what is needed in terms of testing is a way to simulate a "next tick" that a given promise implementation might use. For example $q service in AngularJS doesn't use setTimeout to trigger async resolution of promises.

Other than this I'm not sure what would be your proposal here - make all the promise implementations behave synchronously?

Other though - you might want to look into https://github.com/domenic/chai-as-promised/ to see how a test-helper method might look like.

maksimr commented Jan 8, 2015

@pkozlowski-opensource you are right. In angularjs we have mechanism like $apply/$digest which trigger resolve/reject of promises synchronously, but what if you don't use anuglarjs, how you should write tests?

If I correct understand source code of chai-as-promised, it is still async tests.
I want to do sync tests, because it is more robust, understandable(sync flow) and fast.

Other than this I'm not sure what would be your proposal here - make all the promise implementations behave synchronously?

I want understand why promise A+ always should be asynchronous.

If the primary goal was to eliminate a footgun code like this:

var value = 1;

promise.then(function () {
  value +=1;
});

console.log('expect', value, 'equal', 2);

Then it's a weak argument for me and this behavior only complicates testing now(because is needed in terms of testing is a way to simulate a "next tick").

@maksimr what you are saying is not complete right - AnuglarJS still preserve asynchronous semantic (that is, execute in a different stack frame) while executing in the same JS turn. You seem to be confusing asynchronicity with a give JS context execution - those are not the same concepts.

Now - the most important part - what is your exact proposal? How would you make promises work asynchronously in the production code and synchronously in the test code? Does it even make sense?

maksimr commented Jan 8, 2015

@pkozlowski-opensource

what is your exact proposal?

If I resolve promise immediately then it should call then handler immediately without setTimeout/nextTick in the same JS context.

Contributor

petkaantonov commented Jan 8, 2015

FYI there is a concept of synchronous inspection implemented by promise libraries like bluebird and Q that is meant to solve this problem and more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment