-
Notifications
You must be signed in to change notification settings - Fork 389
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
Socket errors could be more verbose in demos #67
Comments
We made a decision a while back to keep console output to a minimum since it often is abused in JS and can lead to a loss in performance. In place of it, we use EventEmitter2 to keep errors (and other output) event driven for those who are concerned with it ( Lines 380 to 382 in 9938d1b
I believe an error is printed to the console if an error is emitted and it's unhandled. For example, you could do: Ros ros = new ROSLIB.Ros();
ros.on('error', function(error) {
console.log(error);
});
... |
Yeah, a console message is written, but the console message is completely unhelpful (something along the lines of an "unhandled error"). In the case of the socket failure (which is a very fundamental thing), it would be better if it could report that the problem has to do with sockets in a more obvious way. A colleague of mine was trying to use the RobotWebTools demos, but nothing was showing up other than the grid and root coordinate triad. It happens that nothing was showing up because the computer in question was on a wireless network which wasn't allowing the necessary ports through. Since the error message contained no information, it necessitated reading through code, downloading, and running a modified version which used the non-minified js scripts to figure out where the error was coming from. So, since this error can happen even with the demos on the website (which would involve a user's first experience with RobotWebTools), I think an exception should be made to either output a console message, or display a message or alert which very explicitly explains that socket connections aren't being made. If that's still something you don't want to do, then there should at least be a prominent Troubleshooting section in the rosjs README. |
(as an aside, these tools are awesome!) |
👍 Thanks! |
I think adding something to the README about how to handle errors and get error messages is a good idea. The reason its bad to just pop up alerts is that everyone is going to want to handle errors differently, therefore we added the event handling feature. |
Do you think it would make sense to add an additional error handler just to the demo pages, so that if someone is trying them out they will see something prominent? |
Yeah, that's a good idea since it will also explicitly show you how to create handlers. |
Yeah, that'd be great! Thanks Russell. I'm updating the issue title to reflect that. |
+1
Another +1 :):):) |
The demos now contain a status indicator and the wiki tutorials have been updated to show how to register listeners for the I'm not closing this issue because I didn't put a note into the README yet. |
I added the troubleshooting section f11baf0 |
enable markdown parsing in JSDoc and add newlines to ensure proper parsi...
Currently, if rosjs fails to create a socket, the user is presented with an empty error message. Adding a console message that reports that the error had to do with the socket creation would be really helpful to new users:
roslibjs/build/roslib.js
Line 452 in 9938d1b
The text was updated successfully, but these errors were encountered: