Reference implementation: InitialisePromise, step 8 (abrupt completion of executor function)

[timwienk]

I believe I have found a small problem in the reference implementation regarding the part of the InitialisePromise description that explains:

1. If completion is an abrupt completion, then
   1. Let status be the result of calling the [[Call]] internal method of resolvingFunctions.[[Reject]] with undefined as thisArgument and (completion.[[value]]) as argumentsList.
   2. ReturnIfAbrupt(status).

Reference implementation: testable-implementation.js:

    try {
        executor.call(undefined, resolvingFunctions["[[Resolve]]"], resolvingFunctions["[[Reject]]"]);
    } catch (completionE) {
        reject.call(undefined, completionE);
    }

It seems that the line after the catch() is supposed to call resolvingFunctions["[[Reject]]"] rather than reject.

I also noticed that this specific part of the spec (draft) is not tested in the reference implementation's tests, so to complete things, a suggestion for a test to add in test/simple.js:

specify("An abrupt completion of the executor function should result in a rejected promise", function () {
    var promise;

    assert.doesNotThrow(function () {
        promise = OrdinaryConstruct(Promise, [function () { throw new Error(); }]);
    });

    promise.then(
        function () {
            assert(false, "Should not be fulfilled");
            done();
        },
        function (err) {
            assert(err instanceof Error);
            done();
        }
    );
});

I did not know whether to make changes against the master branch or the january-simplification branch, so I opted to just report and suggest, rather than Pull Request. I can Pull Request against either branch if preferred. :-)

[domenic]

Oh, nice catch! Master branch would be great. Lemme delete the other branch.

[timwienk]

In case you hadn't noticed, I submitted a PR for this as #110.

[domenic]

Ya sorry, been running around to conferences for the last few weeks, will merge shortly.

On May 29, 2014, at 10:12, "Tim Wienk" notifications@github.com<mailto:notifications@github.com> wrote:

In case you hadn't noticed, I submitted a PR for this as #110https://github.com/domenic/promises-unwrapping/pull/110.

Reply to this email directly or view it on GitHubhttps://github.com/domenic/promises-unwrapping/issues/109#issuecomment-44535624.

[timwienk]

That's quite alright, no need to apologise at all. I just wanted to make sure it didn't get lost somewhere in the eternal bitfields.

Thanks for merging. :-)