muon icon indicating copy to clipboard operation
muon copied to clipboard

Handle errors during type conversions

Open ImVexed opened this issue 5 years ago • 8 comments

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 avatar Oct 08 '19 15:10 ImVexed

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

goku321 avatar Oct 12 '19 18:10 goku321

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.

ImVexed avatar Oct 12 '19 19:10 ImVexed

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

goku321 avatar Oct 12 '19 19:10 goku321

@goku321 Any progress on this?

ImVexed avatar Oct 26 '19 18:10 ImVexed

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.

goku321 avatar Oct 30 '19 05:10 goku321

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

ImVexed avatar Oct 30 '19 06:10 ImVexed

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

goku321 avatar Nov 10 '19 16:11 goku321

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.

ImVexed avatar Nov 10 '19 22:11 ImVexed