Skip to content
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

run(rsvp, "reject") should not throw trying to dispatchError #14184

Merged
merged 1 commit into from
Sep 2, 2016
Merged

run(rsvp, "reject") should not throw trying to dispatchError #14184

merged 1 commit into from
Sep 2, 2016

Conversation

jasonmit
Copy link
Member

@jasonmit jasonmit commented Sep 1, 2016

Fixes #14182

@@ -6,7 +6,7 @@ let getStack = function(error) {
var stack = error.stack;
var message = error.message;

if (stack.indexOf(message) === -1) {
if (stack && stack.indexOf(message) === -1) {
Copy link
Member Author

@jasonmit jasonmit Sep 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

synonymous with the previous behavior where it will Log/dispatch undefined if the error did not have a stack

@rwjblue
Copy link
Member

rwjblue commented Sep 1, 2016

Seems odd that the test is in runtime, but implementation is in metal. I guess the way your are excercising the bug is via RSVP (which is only in runtime). Is it possible to also add a test in metal?

Also, can you determine what published major/minor this issue was introduced in? We should likely target that version for this BUGFIX.

@jasonmit
Copy link
Member Author

jasonmit commented Sep 1, 2016

Seems odd that the test is in runtime, but implementation is in metal. I guess the way your are excercising the bug is via RSVP (which is only in runtime). Is it possible to also add a test in metal?

I put it where I thought it related with the other tests, I can move to metal though since you're likely right.

Also, can you determine what published major/minor this issue was introduced in?

I don't believe it's in a stable release yet but I will verify.

@jasonmit
Copy link
Member Author

jasonmit commented Sep 1, 2016

Also, can you determine what published major/minor this issue was introduced in?

2.8.0-beta.1 is where it was first introduced.

@rwjblue
Copy link
Member

rwjblue commented Sep 2, 2016

Nah, this test is good here.

Can you prefix the commit with [BUGFIX beta]?

@jasonmit
Copy link
Member Author

jasonmit commented Sep 2, 2016

@rwjblue done

@jasonmit
Copy link
Member Author

jasonmit commented Sep 2, 2016

@rwjblue ready and the one selenium job failing is failing against everyones PR so I'm assuming it's unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants