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

Socket errors could be more verbose in demos #67

Closed
jbohren opened this issue Feb 20, 2014 · 11 comments
Closed

Socket errors could be more verbose in demos #67

jbohren opened this issue Feb 20, 2014 · 11 comments

Comments

@jbohren
Copy link

jbohren commented Feb 20, 2014

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:

this.socket.onerror = onError;

@rctoris
Copy link
Contributor

rctoris commented Feb 20, 2014

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 (

roslibjs/build/roslib.js

Lines 380 to 382 in 9938d1b

function onError(event) {
that.emit('error', event);
}
)

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);
  });

  ...

@jbohren
Copy link
Author

jbohren commented Feb 20, 2014

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.

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.

@jbohren
Copy link
Author

jbohren commented Feb 20, 2014

(as an aside, these tools are awesome!)

@rctoris
Copy link
Contributor

rctoris commented Feb 20, 2014

👍 Thanks!

@rctoris
Copy link
Contributor

rctoris commented Feb 20, 2014

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.

@jbohren
Copy link
Author

jbohren commented Feb 20, 2014

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?

@rctoris
Copy link
Contributor

rctoris commented Feb 20, 2014

Yeah, that's a good idea since it will also explicitly show you how to create handlers.

@jbohren
Copy link
Author

jbohren commented Feb 20, 2014

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.

@jihoonl
Copy link
Member

jihoonl commented Feb 20, 2014

I think adding something to the README about how to handle errors and get error messages is a good idea.

+1

Yeah, that's a good idea since it will also explicitly show you how to create handlers.

Another +1

:):):)

@T045T
Copy link
Contributor

T045T commented Jun 12, 2014

The demos now contain a status indicator and the wiki tutorials have been updated to show how to register listeners for the error, close and connection messages.

I'm not closing this issue because I didn't put a note into the README yet.

@syrnick
Copy link

syrnick commented Jul 9, 2014

I added the troubleshooting section f11baf0

@syrnick syrnick closed this as completed Jul 9, 2014
k-aguete pushed a commit to k-aguete/roslibjs that referenced this issue Oct 21, 2022
enable markdown parsing in JSDoc and add newlines to ensure proper parsi...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants