monteslu / pagenodes

Completely Browser Based IOT Platform
https://pagenodes.com
Apache License 2.0
222 stars 32 forks source link

Consider not swallowing errors in /nodes/core/io/22-websocket.js #25

Open malakada opened 8 years ago

malakada commented 8 years ago

And handle them with intent.

https://github.com/monteslu/pagenodes/blob/master/nodes/core/io/22-websocket.js#L168 https://github.com/monteslu/pagenodes/blob/master/nodes/core/io/22-websocket.js#L179

samrocksc commented 8 years ago

I had actually restructured the server that hosts the websockets server to catch error emitters into the socket stream, we had also had some issues with this bubbling up in the peer2peer connections. With peer to peer though you are dropping from a socket stream into an RTC stream.

What would you recommend on the handling of these(as i am more novice at error handling).

malakada commented 8 years ago

I don't really know this node/module/whatever well enough to have a real recommendation on how to handle this specific situation.

What I could recommend is looking into why this error is thrown, figure out what it means, and if you can recover/fix things. Otherwise, you should log the error to an error log... or in some cases consider just failing right out and crashing everything so that way you can fix the actual upstream bug. Suppressing errors like this leads to weaker code that you don't fix edge cases and bugs/problems with. You certainly don't want to catch it and do nothing. :)

Also, as a side note: console.warn can be buggy in other browsers (like IE), console.log is generally going to be preferred.