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

Promise.reject(reason) often fires and ignores exceptions -- not debug friendly #144

Closed
jparker-hbo opened this Issue Mar 14, 2014 · 1 comment

Comments

Projects
None yet
2 participants
@jparker-hbo

jparker-hbo commented Mar 14, 2014

The docs say "Promise.reject(dynamic reason) -> Promise" but the code does this:

function markAsOriginatingFromRejection(e) {
    try {
        notEnumerableProp(e, "isAsync", true);
    }
    catch(ignore) {}
}
function notEnumerableProp(obj, name, value) {
    var descriptor = {
        value: value,
        configurable: true,
        enumerable: false,
        writable: true
    };
    es5.defineProperty(obj, name, descriptor);
    return obj;
}

So if "reason" is undefined or something that can't have a property defined like a string literal, then it throws and ignores it.

This doesn't crash, but if you're using "pause on all exceptions" in Chrome, it's annoying. Also, then the "isAsync" property won't exist... does that cause problems later?

If you like the existing behavior, then the docs should change to say that "reason" needs to be an object.

@petkaantonov

This comment has been minimized.

Show comment
Hide comment
@petkaantonov

petkaantonov Mar 14, 2014

Owner

By spec a promise rejection reason can technically be a non-error like a string or a number or other absurd value but the non-spec extensions like .error() don't support this. See show/02738964b9a97571a53ee65905b59c4debce57ec/88333ba#diff-88333ba11114e014e87198d04680144d

I guess notEnumerableProp could add a check to avoid throwing for that case

Owner

petkaantonov commented Mar 14, 2014

By spec a promise rejection reason can technically be a non-error like a string or a number or other absurd value but the non-spec extensions like .error() don't support this. See show/02738964b9a97571a53ee65905b59c4debce57ec/88333ba#diff-88333ba11114e014e87198d04680144d

I guess notEnumerableProp could add a check to avoid throwing for that case

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