-
Notifications
You must be signed in to change notification settings - Fork 28
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
semantics of "is-a Future" #41
Comments
@dherman, re:
I don't see how import { promiseBrand } from "promise";
assert(promise && promise[promiseBrand]); is better than
Obviously
I think this might not be worded that clearly, i.e. it confuses the issues of In @erights's formulation, and in Promises/A+ (modulo our upcoming promise-creation spec), only |
Thanks for breaking down the dimensions of the problem. I will argue for simple duck-typing. It is equally bad in practice to fail to recognize something that is a promise, or at least close enough to being a promise that it can be coerced, as it is to accidentally attempt to coerce an object that is not a promise. If an object has a "then" method, it is a close enough approximation to a promise that we can reliably coerce it into a good promise, even with very bad historical promise implementations. If an object has a "then" method but is not a promise and has to be passed through a promise implementation, it is simple enough to make it a property of a wrapper object. @dherman mentions this case where Alice and Bob are third party libraries that do not know of each other. alice().then(function (a) {
return bob(a);
}) Note that Alice and Bob are both necessarily aware that they are in the thenable promise ecosystem. Even today, they would both be aware of the peril. Another case is where Alice is coordinating Bob and Charlie, where Bob is providing objects with then methods that are not promises and Charlie is consuming promises. This puts Alice in a position where she would know to wrap Bob’s objects such that Charlie can consume them. For example, this would occur if one were to pass @polotek’s process wrappers (which have a "then" method but are not promises) to a promise consumer. In this kind of case, there is always an arbiter (Alice) that can mitigate the problem. To change the landscape will require a pretty significant amount of collective political will and a synchronized abandonment of existing implementations, including prollyfills. The arity check is too strong. Promises/A has an arity of 3 and Promises/A+ has an arity of 2. Then there are forwarders that have an arity of 0. Note that any symbol or sentinel value would have to be global enough that every promise implementation, even multiple instances of identical promise implementations, can recognize each other’s promises, and that every promise implementation would need to look for it in the same place, and for migration purposes, install it if it does not exist. There is also no migration path from promises of today and promises that use symbols or modules to solve the problem. The only solution that does not use duck typing is to make migration a non-goal. |
@domenic I think the details come down to how specific the string-key name is. If it's Whereas, if the promise library itself creates and publishes a whole new value, then the library has the right to document the purpose of that value. It's not possible for someone to accidentally create a value with that brand without knowledge of the promises library, because they had to get it from the promises library! And by using the library they're agreeing to obey its contract. If they choose to violate the contract, it's their own fault if fulfilling does something weird. Dave |
@kriskowal A number of good points there. In particular I agree that the arity check is a no go. But I disagree that "anything with a At the end of the day, I agree that it's possible to code your way around any of this, by being careful to wrap values that might possibly conflict with promises. In practice, I expect no one would actually do this, and most code would be susceptible to these bugs. They're just rare enough that no one will bother, and yet when they do happen they'll be really bad. And if you do try to write more robust code you'll pay a performance penalty for it. Your points about migration and interoperability are good. Even if the web platform publishes a nonce that everyone can use, legacy code won't be able to use it, and it may not be possible to polyfill. I guess I'm not yet convinced that it's impossible to address migration and branding, but I'd have to think about it. Dave |
@dherman the main point I was making was that accidents are possible with one-level-deep brands, e.g. |
I want to repeat a point from over in #40 (comment) that I don't see being addressed. If there is branding, it would mean that the following code does not work: var promise = aplusCompliantPromise.then(() => jQueryPromise);
var promise = new Promise(resolve => resolve(jQueryPromise)); i.e. the promise becomes fulfilled with Instead you need to provide an assimilation function, e.g. var promise = aplusCompliantPromise.then(() => Promise.from(jQueryPromise));
var promise = new Promise(resolve => resolve(Promise.from(jQueryPromise))); I want to make sure everyone who is pro-branding recognizes this cost and thinks it is worth it. I am on the fence, personally (and thus leaning toward the status quo of no-branding). |
@domenic Good points all around. The latter point makes me sad, and I think it's about the same point @kriskowal is making: "like it or not, it's the cowpath." If I lose this argument I suppose I'll live to see another day, I just may have to curse it along with Dave |
A quick note, written only after skimming since I need to leave in a moment. From skimming, this thread seems to be missing the crucial distinction between thenables and promises (ka "futures"). Promises should be recognized only by unforgeable branding/trademarking. But thenables, by definition, can (and should) only be recognized by duck typing. The only place where a promise library should duck type to recognize a thenable is when deciding whether to assimilate. https://code.google.com/p/google-caja/source/browse/trunk/src/com/google/caja/ses/makeQ.js#204 does both of these correctly. |
@erights That doesn't affect my argument. My point is that making a decision to recognize a thenable is the problem. Apologies for using the wrong terminology. Dave |
I have now read through this thread carefully, and I think we're still missing the distinction between promises (aka futures) and thenables. @dherman I agree that the points you raise apply even just to the assimilation test by itself. But in the context where promises are reliably branded, the issue is much less bad than others might imagine reading this thread. @dherman @domenic @kriskowal @slightlyoff The branding must be a reliable indicator that the value in question obeys the promise contract. @dherman writes: "If they choose to violate the contract, it's their own fault if fulfilling does something weird." Let's rephrase that as "If an attacker chooses to violate the contract, it's their own fault if that misleads the victim into doing something weird." Let's remember where this promise design originated and what its original purpose was: to protect the invoker/victim from the plan interference hazards that arise from caller/callee interleaving http://www.erights.org/talks/promises/paper/tgc05.pdf http://www.infoq.com/presentations/Secure-Mashups-in-ECMAScript-5 http://qconsf.com/dl/qcon-sanfran-2011/slides/MarkS.Miller_RemainingHazardsAndMitigatingPatternsOfSecureMashupsInEcmaScript5.pdf . In particular, we need the following guarantee:
where Q is the trusted Q, but 'something' is obtained from an untrusted client, must not cause any of that client's untrusted code to run during the present turn. This becomes even more important starting in ES6 because proxies introduce far more potential interleaving hazards, raising the priority to have a way to reliably protect against these. Even if something is a proxy, the code above must not trigger any of its traps during the current turn. That's why the code at https://code.google.com/p/google-caja/source/browse/trunk/src/com/google/caja/ses/makeQ.js#204 only does synchronously operations that proxies don't trap. Any why the assimilation check is postponed till a later turn, because it is impossible to test a duck type without triggering a trap. (Note to @kriskowal : Last I looked, the assimilation test in your Q implementation was not safely asynchronous in this sense.) |
One (unsolicited) data-point about duck-typing to test (for promises or thenables or whatever): I have two async-flow-control libs (in use on a number of projects) which take a somewhat abstracted approach to promises that's more high-level task-oriented than generic promise utility. One is asyncGate and the other is asyncSteps... the main difference being whether the API processes in "parallel" or "serial". Both libs have a By some definitions of what I read in this thread, these libs would be the counter-example that just testing for a |
Another solution might be to allow pages to provide a callback which tests if a given object is a Future. So something like: Future.addIsFutureHook(function(v) {
return !!v && typeof v.then === "function";
} This way the Promise/A+ library or any other library can install their own hooks and test as conservatively as they can. Multiple hooks would be allowed and if any of them claim that a value is a Future, then it's considered a Future. Over time hopefully pages will migrate to using just built-in promises and the hook ends up unused and could potentially even be removed. |
@sicking Do you mean, to test if it is a future/promise, or to test if it is a thenable? What is the purpose of the test? |
As far as I can tell, what we're mostly concerned with here is to test if something is thenable. But since the subject of the thread is "is-a Future" I used that terminology. But "thenable" is more clear. Generally speaking I think providing a function like It might additionally be useful to provide a |
The purpose of a "thenable" test would be to allow people to write APIs which accept values either synchronously by passing the value directly, or asynchronously by passing a thenable object. The Cursor proposal that I made quickly turned out to need this ability, and I would expect other places to need this too once thenables get used more consistently across the platform. |
@sicking Why make the thenable test configurable? Since an assimilation convention makes the thenable test a coordination point among separately developed promise/future libraries, aren't we all better off if we jointly agree on a simple adequate convention? Given than a thenable test must be duck anyway, and thus must imply all the hazards @dherman raises, I think the test you wrote
is the one we should settle on. Anything more "precise" only adds complexity without significantly reducing the hazard. Worse, an extensible thenable test, where each library adds their own extension to Future.addIsFutureHook, would mean that library A, even in the absence of malice, might extend the thenable test so as to break a working relationship between libraries B and C. |
The point of a configurable testable test is to avoid exactly the type of breakage you are talking about. While I suspect that The whole point of the isThenable test is to check if you can call So I don't think that library A would be able to break interactions between B and C any more than the simple typeof test would. |
Library B and C might count on D being classified as thenable and count On Fri, Mar 8, 2013 at 12:23 PM, Jonas Sicking [email protected]:
Text by me above is hereby placed in the public domain Cheers, |
Please provide a more concrete example. I'm also not convinced that what you are describing is more likely than that someone creates an object with a function named 'then' but which is not future/promise related at all. That is the big concern with the alternative approach. |
To put it another way, the risk seems higher that This because |
library B:
library C:
library A:
If you don't like this example because the added hook function isn't realistic, please show an example of a hook function you do think is realistic and I'll show you a program in this pattern broken by it. |
That hook is very broken. You should only claim that an object is "thenable" if it has a If an object is not implementing that contract, then you should not claim that it's thenable. The purpose of the thenable test is to enable people to write code like: x = someLibrary.doStuff();
if (Future.isThenable(x)) {
x.then(processResult);
}
else {
processResult(x);
} This code would obviously not work if This pattern is needed both by the internals of the Future code, and will surely be needed in many other places. See my cursor proposal for example. Two examples of realistic hooks: Future.addIsThenableHook(function(v) {
return !!v && typeof v.then === "function" && v.then.isapluspromise === true;
});
Future.addIsThenableHook(function(v) {
return !!v && typeof v.then === "function" &&
typeof v.catch === "function" &&
v.myhomegrownpromiselibrary;
}); |
After A executes all these addIsThenableHook calls, is an object considered thenable iff it passes all of them or iff it passes any of them? I'm still puzzled about what you're proposing. If "all", then D would no longer be considered thenable, breaking the B-C relationship. |
it's thenable if it passes "any" of the tests. I'm proposing that the only "preinstalled" test is one that only returns true for objects created using What test are you envisioning that B and C would register that would make this system not work? The intent is that a revised version of Promises/A+ would set some identifiable flag on all promises that it creates and then register using the hook to classify all its promises as to make them compatible with the Future API. |
The more likely outcome of this, IMO, is that people keep using their Promises/A+ library and whenever they see a DOMFuture, they wrap it in order to get the expected behavior, just like they do with jQuery promises today. On Mar 9, 2013, at 0:31, "Jonas Sicking" <[email protected]mailto:[email protected]> wrote: it's thenable if it passes "any" of the tests. I'm proposing that the only "preinstalled" test is one that only returns true for objects created using new Future (as well as created though built-in APIs like revised IndexedDB API etc). What test are you envisioning that B and C would register that would make this system not work? The intent is that a revised version of Promises/A+ would set some identifiable flag on all promises that it creates and then register using the hook to classify all its promises as to make them compatible with the Future API. — |
@sicking I am assuming that B and C had already registered the test
as that is the generally accepted convention. If so, then the additional tests registered by A have no effect. |
@sicking Very specifically for this example, C would register this conventional hook, in order to recognize objects such as B's D object above as a thenable. Neither of your more specific hooks by themselves would have been adequate to recognize B's D as thenable. |
Mark: I feel like we are going in circles. The goals that I have are:
new Future(function(r) {
var x = functionReturningCustomPromise();
r.resolve(x);
}) or functionReturningFuture().then(function(v) {
return functionReturningCustomPromise(v);
}).then(function(v) {
console.log(v);
}); It would further be great if people wanting to integrate with code which uses promises or Futures could write code like: var x = doSomething();
if (<some function testing if an object is "thenable">(x)) {
x.then(myHandler);
}
else {
myHandler(x);
}
My proposal for resolving 1-3 was to add the following two functions: Future.isThenable = function(x) {
if(!x) {
return false;
}
if (x.[[Class]] === "Future") {
return true;
}
return @callbacks.some(function(callback) {
try { return callback(x); }
return false;
});
};
Future.addIsThenableHook(x) {
@callbacks.push(x);
} This would surely result in people adding completely malfunctional hooks. It's also the case that people would add hooks that simply test If we add a test like Adding a If you simply don't care about one of the points above, that's certainly a valid answer, just one I don't agree with. |
@sicking Hi Jonas, well put. But I think a configurable thenable test will in practice create all the same problems you're trying to avoid, because some popular libraries will add broad tests. And configurability will create more problems, because these additional tests will disrupt the assumptions of other co-loaded libraries. So, in light of this, I'm willing to give up on 3 because I believe that a configurable thenable test will sacrifice it anyway. And since it will be gone either way, best to codify one conventional way. |
I'd personally give up 1 rather than give up 3. Like @domenic says, people can always manually convert between different promises. |
Currently the prolyfill uses this code for testing whether a value is a Future:
The issue with "duck testing" is that, as @domenic raised in Issue #13, the type of a fulfilled future is not fully polymorphic: you must never fulfill with a future that you intended to be a completed value. Bottom line: there needs to be a super-clear and ideally super-simple definition of what it means to be a future.
I'll pitch a few alternatives here, but I make no claim to exhaustiveness. Other suggestions welcome.
Approach: a "Future"
[[Class]]
with anisFuture
testUsing
instanceof
is traditionally brittle because when multiple frames interact, they can have multiple, mutually incompatible versions of a given "class." That's why ES5 introducedArray.isArray
as a way of testing whether something has the[[Class]]
"Array", which is robust across frames, even when they have differentArray
constructors. So an analogous approach would be to introduce a new "Future"[[Class]]
and anisFuture
test that inspects the class. This is very clear and very robust, but has the downside that you can't create futures conveniently with an object literal.Approach: a
Symbol
to definitely brand futuresAnother approach would be to create a unique symbol (a new ES6 feature for creating opaque unforgeable property keys) that's shared in the standard library and identical across all frames. Then to create a future you would have to have this symbol, a la:
Approach: a string key to "probabilistically" brand futures
A lower-tech approach is to have some sort of published key that is simply a standard string key, but perhaps distinctive enough that it's highly unlikely (though still possible) that someone could accidentally create one. For example, Promises/A+ discussed alternatives like
p.then.aplus
orp.isPromise
.This approach is certainly the easiest, but I would hope we could do better when we have the opportunity to define this in the web platform and/or ES standard. What's particularly galling about it is that it effectively declares that whatever public key it's using is once and for all permanently associated with this type—that's global pollution of the entire JavaScript language. Scumbag futures!
Approach: type-and-arity check (again, probabilistic)
We could always just do what the prolyfill currently does. Again, while it's unlikely that someone would accidentally fulfill a value that is not intended to be a future but looks like one, that doesn't mean it can't happen. For example, you might be writing future-aware code that nevertheless fulfills with the result of calling some third-party code, and passes it back over to some other third-party code. And those third-parties may not be future-aware. If they happen to be using values that look like futures, not only will the program be buggy, it'll be buggy in badly unpredictable ways.
See also:
Dave
The text was updated successfully, but these errors were encountered: