Uncaught error about node-red-contrib-ads-client HOT 9 CLOSED

Here I have a full log of a crash on debug level 2:
Node-red.log

from node-red-contrib-ads-client.

Thanks for reporting, again!

It seems that the PLC isn't available (Device system manager state read failed) and the ads-client disconnects, as there is nothing to connect to (except router).

However, in ads-client, the connected is set to true after getting ADS port from router. So it says "connected" too early. See line 392: https://github.com/jisotalo/ads-client/blob/master/src/ads-client.js#L392 The connected flag should be set to true near line 458 so it's not too early. This needs to be fixed.

From your log, we can see that the subscribe node is trying to subscribe at the same time the ads-client is disconnecting. However, there should be no uncaught exception, as all calls to the _socketWrite() are inside try...catch. Perhaps the reason is that the socket.on('error', this._internals.socketErrorHandler) line, which adds error event listener to socket, is after connected = true line..? Maybe try..catch isn't working with socket.write and the on('error') is the only way.

Edit: Yeah it might be it. There are those few lines during the error listener isn't active (lines 392..420).
https://stackoverflow.com/a/17637900/8140625

from node-red-contrib-ads-client.

Actually it's not too easy. Everything after that connected = true line depends on a valid connection.

Everything works if the connect() is waited to finish, but if, like in Node-RED, tries to do stuff already during connecting, things get dirty.

Needs some rethinking.

from node-red-contrib-ads-client.

If we make the following change:

this.isConnected = () => this.adsClient === null ? false : this.adsClient.connection.connected && !this.isConnecting()

And use the proposal from my pull-request #11 to be able to handle concurrent connect calls. Wouldn't that solve it? You basically as you said wait for the connect() function to end.

from node-red-contrib-ads-client.

That makes sense. Good idea.

I'm also working on update to the ads-client, so that the connected is set after we are really connected. But this isConnected() change won't harm anything even after that.

from node-red-contrib-ads-client.

I installed version v1.3.0 and after multiple times starting and stopping the system manager, I got the ECONNRESET again.

ads-client: WARNING: Connection was lost. Trying to reconnect...
ads-client: WARNING: Local AMS router state has changed to START. Reconnecting...
ads-client: WARNING: Connection was lost. Trying to reconnect...
ads-client: WARNING: Local AMS router state has changed to START. Reconnecting...
ads-client: WARNING: Connection was lost. Trying to reconnect...
8 Jul 16:41:37 - [red] Uncaught Exception:
8 Jul 16:41:37 - Error: read ECONNRESET
    at TCP.onStreamRead (internal/stream_base_commons.js:209:20)

Similar to #12 I see multiple warnings regarding Reconnect/Lost/...
I wouldn't say it multiplies exactly each time I reconnect, but I'm getting more and more warnings. And it's definitely more than the amount of ads nodes in the flow.

For reference:
I have a TwinCat program with TC_Stop functionblock to stop the systemmanager, and following flow:

from node-red-contrib-ads-client.

I'll try to find the reason.

What is the output if you set the debugging level to something higher? (1 or 2)

from node-red-contrib-ads-client.

I just updated ads-client to 1.11.1.

I managed to get the system work better, couldn't get any issues anymore with my Raspberry Pi.

However, after releasing it and testing one more time, I once managed to get another problem. The client connected again and again, even though it connected successfully in between. But now I can't seem to cause that again, will keep trying. That happened when I first started Node-RED so that PLC was in config mode. Then I started to change between run and config and BAM.

So if you like, please test if updating ads-client to 1.11.1 helps for your problems.

ads-client [1.11.1] - 10.07.2021

Changed

• Tried to fix edge cases where unhandled exception was thrown (node-red-contrib-ads-client, issue #13)
  • Example: When PLC was restarted (config mode) multiple times, sometimes an exception was thrown
• Improved operation during connection faults
• Fixed typos (See issue #71)
  • byteArrayToAmsNedIdStr --> byteArrayToAmsNetIdStr
  • amsNedIdStrToByteArray --> amsNetIdStrToByteArray

from node-red-contrib-ads-client.

Closing this as no more new reports & ads-client version 1.12.0 should also work on these issues.

from node-red-contrib-ads-client.