-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
Fix error handling, Issue #937 #1055
Conversation
This is what we were looking for! And it looks like it may be revealing a flaw in our code. We'll have to look into the test failure to see what's going on. We'll try to do that soon and let you know. Thank you! |
src/utils/valueFixers.js
Outdated
@@ -37,7 +37,7 @@ const toNumber = function (numberOrString) { | |||
|
|||
let trueOrError = isNumberlike(numberOrString); | |||
|
|||
if (typeof trueOrError === `object`) { | |||
if (typeof trueOrError instanceof Error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it now! More needs to be changed here. typeof Error === 'object'
, so when isNumberlike()
returns an Error
, this will come out false and won't behave the way we want.
Not sure I described that clearly. Feel free to ping me if I need to clarify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see now. The line should be written as if (trueOrError instanceof Error)
since typeof trueOrError
will always return object
and object instanceof Error
will always return false
.
I can work on making that fix and resubmitting.
Locally passing. Re-running the Travis PR build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks a ton!
Let me know if I misunderstood and this fix is not as simple as what I did here.