new $q(executor) does not catch exceptions

[lapo-luchini]

The following code prints UErr.

    try{
        $q(function (res, rej) {
            throw new Error('Wrong.');
        }).then(function () { $log.info('OK?');
        }).catch(function (e) { $log.info('Err', e);
        });
    } catch (e) { $log.info('UErr'); }

I'm not sure I understand the ES6 specs but in BlueBird (new Promise(executor)) and in Q (new Q.Promise(executor)) this prints Err [Error: Wrong.].

[lapo-luchini]

Does not seem related to #3174 and could probably be solved in q.js#L558 like this:

try {
    resolver(resolveFn, rejectFn);
} catch (e) {
    deferred.reject(e);
}

[petebacondarwin]

I think you might be right. From the ES6 spec:

10. Let completion be Call(executor, undefined, «resolvingFunctions.[[Resolve]], resolvingFunctions.[[Reject]]»).
11. If completion is an abrupt completion, then
  a. Let status be Call(resolvingFunctions.[[Reject]], undefined, «completion.[[value]]»).
  b. ReturnIfAbrupt(status).

My reading of this is that if the call of executor returns abruplty, e.g. throws, then it should call the reject function.

@caitp - you know about this stuff. Does that sound right to you?

[caitp]

@petebacondarwin technically yes, per spec, this should be a rejection.

It shouldn't be a rejection for a lot of better reasons (early errors are less likely to be missed if they explicitly throw).

Native promises in Chromium work around this now by reporting uncaught rejections (and Bluebird does this as well), but it wasn't popular with Igor.

I think it's better to leave it as-is, because A) we avoid the try/catch deopt, and B) we make it possible for developers to catch their early errors.

[petebacondarwin]

I agree with @caitp - we should leave it throwing. It is simple enough to work around this in application code if you wish by wrapping your own executor function:

    try{
        $q(function (res, rej) {
            try {
              throw new Error('Wrong.');
            } catch(x) {
              rej(x);
            }
        }).then(function () { $log.info('OK?');
        }).catch(function (e) { $log.info('Err', e);
        });
    } catch (e) { $log.info('UErr'); }

[petebacondarwin]

@IgorMinar do you want to chime in here too?

[caitp]

I'm not strongly opposed to a patch to fix it though, I just worry that it might lead people to ignore incorrect code (or not realize why their code doesn't work)

[lapo-luchini]

If it is chosen not to adhere to the ES6 specs, I'd suggest to add some comment in the documentation, to avoid the "unexpected" factor. (or at least, unexpected behavior could still bite people, but then they could check the docs and find the reason)

[IgorMinar]

@caitp just to clarify, I wasn't a big fan of the implementation we had. I do like the feature.

I think for now we should document the deviation from the spec as well as the workaround. In 1.5 we can take a look at this and see if we can do better.

[stucox]

Raised PR #13101 to document this.