cruise-automation / webviz

web-based visualization libraries
https://webviz.io/
Apache License 2.0
2.03k stars 412 forks source link

Error when Publisher component is rendered before rosClient connection hook runs #534

Open JanOlencki opened 3 years ago

JanOlencki commented 3 years ago

Hi,

I found an issue in the webviz-core, when the Webviz are connected to the rosbridge via a websocket. If the Publisher component is rendered before the rosClient (roslib.Ros instance) connection hook runs at RosbridgePlayer.js:98 an error listed below occured. It's caused by null reference at RosbridgePlayer.js:331.

Uncaught TypeError: Cannot read property 'idCounter' of undefined
    at Topic.advertise (roslib.js:3360)
    at Topic.publish (roslib.js:3405)
    at exports.default.publish (RosbridgePlayer.js:349)
    at exports.default.publish (index.js:462)
    at exports.default.publish (OrderedStampPlayer.js:153)
    at Object.publish (index.js:251)
    at Publisher.publish (Publisher.js:50)
    at index.js:180
    at index.js:154
    at Object.ka (react-dom.production.min.js:15)

Test case

jaguardo commented 3 years ago

I think I have witnessed this effect without an error, but also without a notification that the publish isn't working (simply no feedback of a published message). I've witnessed in both in the current webviz Publish panel and in the @jreyesr PR (Teleop panel), periodically the Publish msg doesn't go through.

Workaround, for both publish panels, if the topic is removed/changed to something else (anything, doesn't even have to be a current topic) and then changed back it will start working.

jreyesr commented 3 years ago

Yes, this bug is especially obnoxious because it shows no feedback on the UI (but there should be an error message on the console, since it's attempting to access a property of an undefined var). I have created a PR that should fix it. I'll wait for feedback on it, then (hopefully) this will be fixed.

vidaaudrey commented 3 years ago

Thank @jreyesr for reporting and trying to fix the issue. Look forward to reviewing the PR.