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

Transaction should not reject with undefined. #1970

Merged
merged 1 commit into from Mar 14, 2017

Conversation

Projects
None yet
2 participants
@wubzz
Collaborator

wubzz commented Mar 14, 2017

No description provided.

@wubzz wubzz referenced this pull request Mar 14, 2017

Closed

Empty error #1966

this._resolver(value)
}
if (status === 2) {
if(isUndefined(value)) {

This comment has been minimized.

@joepie91

joepie91 Mar 14, 2017

Contributor

Is the intention to only do this when rejecting with undefined? I'm not sure under what circumstances this issue could occur (as I haven't followed the issue closely), but other non-Error rejection values such as numbers or strings would have the same "missing stacktrace" issue.

This comment has been minimized.

@wubzz

wubzz Mar 14, 2017

Collaborator

Yes, this is intentional. By definition undefined is the only javascript type that can be considered non-existant. Null, Strings, Numbers, Objects, Errors, Buffers, or any other type of value is still valid.

For example some people prefer to use strings (why is beyond me):

try {
    throw 'poop'
} catch(e) {
    console.log(e, typeof e);
    //poop string
}

So in cases where rollback is called with an explicit value other than undefined, that value will be used in the reject handler.

@wubzz wubzz merged commit b722753 into tgriesser:master Mar 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

elhigu added a commit that referenced this pull request Mar 25, 2017

elhigu added a commit that referenced this pull request Mar 26, 2017

Fixed transaction promise mutation issue (#1991)
* Fixed transaction promise to not be overridden

* Disabled failing test case until #1970 feature if fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment