Improve error message of failed requests

[flacohenao]

So... I have an unreadable Area (DB1), is not defined on my PLC blocks.

itemGroup.addItems(["DB1,X0.0"]);
            itemGroup.readAllItems()
                .then(result => {
                    console.log('result: ', result);
                })
                .catch(err => {
                    console.log('ERRORS: ', err);
                });

In this case, note that I have the REGEX_NODES7_ADDR in good shape, but, DB1 is not reachable.

So, basicly:

ERRORS:  Error: Disconnected
    at C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\nodes7\src\s7connection.js:361:21
    at Map.forEach (<anonymous>)
    at S7Connection.destroy (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\nodes7\src\s7connection.js:360:28)    at S7Endpoint._destroyConnection (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\nodes7\src\s7endpoint.js:263:26)
    at S7Endpoint._disconnect (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\nodes7\src\s7endpoint.js:325:14)    at S7Endpoint._onConnectionError (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\nodes7\src\s7endpoint.js:385:14)
    at S7Connection.<anonymous> (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\nodes7\src\s7endpoint.js:244:48)
    at S7Connection.emit (events.js:311:20)
    at S7Connection.EventEmitter.emit (domain.js:482:12)
    at S7Connection._onParserError (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\nodes7\src\s7connection.js:142:14)
    at S7Parser.<anonymous> (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\nodes7\src\s7connection.js:98:44) 
    at S7Parser.emit (events.js:323:22)
    at S7Parser.EventEmitter.emit (domain.js:482:12)
    at errorOrDestroy (internal/streams/destroy.js:108:12)
    at onwriteError (_stream_writable.js:456:5)
    at onwrite (_stream_writable.js:483:5)
    at S7Parser.afterTransform (_stream_transform.js:98:3)
    at S7Parser._transform (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\nodes7\src\s7protocol\s7parser.js:478:21)
    at S7Parser.Transform._read (_stream_transform.js:191:10)
    at S7Parser.Transform._write (_stream_transform.js:179:12)
    at doWrite (_stream_writable.js:441:12)
    at writeOrBuffer (_stream_writable.js:425:5)

The thing is, the connection is actually being disconnected but the error doesn't mean anything, so the user cant know why..

Also, I think that, the error is not a connection fault, the connection is good! this is user fault!!.. so why drop the connection? (I know maybe this was thinked based on the node-Red module). So, having to handle with the reconnection and also telling the user what happened (coz the error doesn't mean anything) is kinda harsh...

I propose to take a look at the #105 specially this:

The second one: (The most fancy and amazing one) Maybe, the addItems() method could just handle that exception and... when he adds it to the list:

let item = new S7Item(tag, addr); this._items.set(tag, item); can add a null or an undefined value instead of the ITEM, so... when the readAllItems() function is executed, you can just ignore the items with null or undefined values and then... in the success of the promise we coud recive some response like:

result: { 'DB2.X0.0': null, 'DB2,X0.1': true }

That would be awesome, coz, instead of managing reconections and stuff we can advise the user that THAT particular variable couldn't be read.. we can even attach the reason like:

result: { 'DB2.X0.0': {value: null, reason: (the reason why)}, 'DB2,X0.1': {value: true} }

Because, at the end, this is not connection fault! So whit that, we can have a very generic library!

If that solutions is not suitable, well.. I guess we just could specify the error?

[gfcittolin]

The connection doesn't get dropped by us in such cases. We only should drop the connection when we can no longer guarantee that we can keep the current connection state. This includes any socket error or any parser error (e.g. a malformed message). Any valid message that includes an error code should only cause the calling action to throw, without breaking anything else.

That said, by the stack trace provided, we can see that the connection is being dropped because the node couldn't correctly parse the incoming message. It looks like this is the same cause as in #107.

What we should improve here is to give the right original cause to the exception instead of the generic "Disconnected" one (even though the connection indeed got disconnected, because of the reasons above).

This is the exception thrown by the readAllItems() right? What error did you get from the S7Endpoint#error event?

[flacohenao]

This is the exception thrown by the readAllItems() right? What error did you get from the S7Endpoint#error event?

Yes!, the exception is thrown by readAllItems() indeed.

and yes, the connection is getting droped

this is the error on the 'error event:

Error: Parser overflow reading response data section [22] > [18]
    at S7Parser._parseResponseData (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\nodes7\src\s7protocol\s7parser.js:337:20)
    at S7Parser._transform (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\nodes7\src\s7protocol\s7parser.js:465:37)
    at S7Parser.Transform._read (_stream_transform.js:191:10)
    at S7Parser.Transform._write (_stream_transform.js:179:12)
    at doWrite (_stream_writable.js:441:12)
    at writeOrBuffer (_stream_writable.js:425:5)
    at S7Parser.Writable.write (_stream_writable.js:316:11)
    at ISOOnTCPClient.ondata (_stream_readable.js:714:22)
    at ISOOnTCPClient.emit (events.js:311:20)
    at ISOOnTCPClient.EventEmitter.emit (domain.js:482:12)
    at addChunk (_stream_readable.js:294:12)
    at readableAddChunk (_stream_readable.js:275:11)
    at ISOOnTCPClient.Readable.push (_stream_readable.js:209:10)
    at ISOOnTCPClient._incomingData (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\iso-on-tcp\src\client.js:199:26)
    at ISOOnTCPParser.<anonymous> (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\iso-on-tcp\src\client.js:81:43)
    at ISOOnTCPParser.emit (events.js:311:20)
    at ISOOnTCPParser.EventEmitter.emit (domain.js:482:12)
    at addChunk (_stream_readable.js:294:12)
    at readableAddChunk (_stream_readable.js:275:11)
    at ISOOnTCPParser.Readable.push (_stream_readable.js:209:10)
    at ISOOnTCPParser.Transform.push (_stream_transform.js:152:32)
    at ISOOnTCPParser._transform (C:\Users\ACER\Documents\NODE PROJECTS\tot-bend\node_modules\iso-on-tcp\src\parser.js:180:18)

[gfcittolin]

Definitely the root cause seems to be the same as in #107. So the behavior here is correct. We received a packet we don't understand, and therefore it's not safe to continue the connection. So let's drop the connection, interrupting any pending job, and try again.

We can focus the parser problem on the other issue then

[flacohenao]

Perfect!