Ractive.Promise pass all tests, but behaves incorrectly.
Closed this issue · 4 comments
Ractive.Promise promises implementation pass all tests, but fails to convert exceptions to rejections. Here is the test code:
var promisesAplusTests = require("promises-aplus-tests");
var Promise = require('ractive').Promise;
//var Promise = require('promise');
//var Promise = require('q').Promise; // get stuck at tests
//var Promise = require('when').Promise;
var adapter = {
resolved: Promise.resolve,
rejected: Promise.reject,
deferred: function() {
var res, rej;
var p = new Promise(function (resolve, reject) {
res = resolve;
rej = reject;
});
return {
promise: p,
resolve: res,
reject: rej
};
}
};
var throw_test = function () {
var p = new Promise(function (resolve, reject) {
throw new Error('Bad happens');
});
return p;
};
if(0) promisesAplusTests(adapter, function (err) {
if(err) console.log('Errors:', err);
});
if(1) throw_test().then(undefined, function (err) {
console.log('Error in promise:', err);
});
Other three implementations (commented-out in code above) reject the promise, which I believe is correct.
So, should Ractive.Promise fail compliance test?
@svbatalov Technically, there is no Promises/A+ specification for the Promise
constructor. These tests are designed only to test then()
's compliance with http://promisesaplus.com. So, even though the test suite does require some way of creating promises in order to test then
, I think this particular case is outside the scope of the tests. IOW, since your implementation passes the tests, it is Promises/A+ compliant :)
In practice, as you observed, most libraries that implement a Promise
constructor do catch those exceptions and reject the newly constructed promise. The ES6 Promise spec requires that as well. So I'd def say, though, that adding a try/catch and rejecting would be the way to go.
Thanks!
Thanks a lot for the clarification @briancavalier - we've fixed it
@Rich-Harris Cool, and no problem at all.