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

Handle errors during type conversions #9

Open
ImVexed opened this issue Oct 8, 2019 · 8 comments
Open

Handle errors during type conversions #9

ImVexed opened this issue Oct 8, 2019 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@ImVexed
Copy link
Owner

ImVexed commented Oct 8, 2019

In many places throughout the code we have to type cast to and from cgo types.

Currently (ex.), we're not handling errors during those conversions and will likely lead to panics during edge cases.

@ImVexed ImVexed added the enhancement New feature or request label Oct 8, 2019
@ImVexed ImVexed self-assigned this Oct 8, 2019
@goku321
Copy link

goku321 commented Oct 12, 2019

Hi @ImVexed if you need help with this one, I can try my hands on this.

@ImVexed
Copy link
Owner Author

ImVexed commented Oct 12, 2019

That'd be appreciated @goku321 I'm tied up until next weekend but should be able to review any pull request related to this in the meantime.

@goku321
Copy link

goku321 commented Oct 12, 2019

Great @ImVexed ! I'll start getting familiar with the code.

@ImVexed
Copy link
Owner Author

ImVexed commented Oct 26, 2019

@goku321 Any progress on this?

@goku321
Copy link

goku321 commented Oct 30, 2019

Hi @ImVexed I moved to a different city few days back and just getting settled here so couldn't make progress on this. If it's okay, I can work on it this coming weekend. Thanks for your patience.

@ImVexed
Copy link
Owner Author

ImVexed commented Oct 30, 2019

@goku321 Absolutely! No worries. Was mainly just a reminder in case it fell off your radar and others were interested in picking it up.

@goku321
Copy link

goku321 commented Nov 10, 2019

Hi @ImVexed I have made a couple of changes and would like to get your inputs just to know if I'm heading in the right direction or not. Here's the link to the commit

@ImVexed
Copy link
Owner Author

ImVexed commented Nov 10, 2019

That code won't work the way you're thinking. JSValueMake* functions all return JSValueRef if I remember correctly. But more than that there's no need for us to ascertain their type.

I'm more concerned with places like https://github.com/ImVexed/muon/blob/master/muon.go#L215 and https://github.com/ImVexed/muon/blob/master/muon.go#L327 where we are blindly casting objects which will likely end in runtime panics given edge cases.

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

No branches or pull requests

2 participants