Add error codes to exceptions

[gfcittolin]

As discussed in https://github.com/plcpeople/nodeS7/issues/107#issuecomment-625587530, we should expose error codes on errors thrown, so we can make error handling easier for the user.

Some considerations:

• Errors originating from the PLC already have an error code that we can expose directly
• Errors from the library itself need to be defined
• We need to model it as there's no conflict between any possible error returned by the PLC and errors we may generate

[gfcittolin]

We can define a custom NodeS7Error class extending from Error that holds the error data. The problems arises on how to better differentiate errors returned from the PLC from errors that we generate internally

I see some options there:

1. Differentiation by data type: This is my preferred method. We can have a single property called code, and set all error codes there. The error codes of the library would be a string code, while the errors from the PLC would be numbers. The constants file have a description of what the numbers mean. Pros: Everything is unified into a single property. Easy to use and lookup. Cons: mix of data types into the same property (number|string)

2. Differentiation by data range: Same as above, but the library errors would be negative numbers. We would have only numbers, but then we'd probably need an additional mapping of numbers to their description, plus the probability of number collisions, although we don't have negative error numbers on the PLC yet (as far as we know)

3. Differentiation by property name: We could have different properties, one for PLC-returner errors, other for internal errors. Error causes would be clearly separated, but it could require checking two properties for errors

@flacohenao your input would be appreciated!

[flacohenao]

So..

I kinda like the first approach with a mix of the third. It is true, that the PLC already has its own error code numbers and that's fine... the error of the library can indeed be strings.

When I say that we could have a little mix with the third option is beacause, in fact there are some generic errors that can be interpreted very easy based on this String code, but, for example the transient errors (just to give you an example) even though the cause of them may be the same (in some cases) would be very nice not just to tell the caller what causes the error but in wich property is failing specifically..

So maybe we don't need to check two properties for errors always, but just when it's really usefull.

Also, I really like the first approach because remember that there are some other errors that already are well thrown like the connection layer errors with they respectively error code, and normally those codes are strings...

And... There may be others that have nothing to be with nodeS7 but with the way programmer do the stuffs but in any case those don't matter to us!

let me know!

[gfcittolin]

Also, I really like the first approach because remember that there are some other errors that already are well thrown like the connection layer errors with they respectively error code, and normally those codes are strings...

Exactly :) What I'm using as reference is Node.JS Errors

When I say that we could have a little mix with the third option is beacause, in fact there are some generic errors that can be interpreted very easy based on this String code, but, for example the transient errors (just to give you an example) even though the cause of them may be the same (in some cases) would be very nice not just to tell the caller what causes the error but in wich property is failing specifically..

I didn't quite get what you mean there. In the first method, we'd have e.g. e.code, if it's a number it comes from the PLC. If it's a string it comes from the library. At least in theory, an Error would have only a single root cause, and then we set the e.code property accordingly. What I mean with option 3 is to have e.g e.code for library errors as a string and e.g. e.errno for PLC errors, but we wouldn't have both set at the same time.

[flacohenao]

Mhh.. What I mean is... practiclly all the nodeS7 erros will fit on the first approach... But when I think is not actually handy is on the transient Errors like the readAllItems() method even though the cause of those erros is actually specific!

So, let me explain myself:

for example when an area is not reachable, like we have discuss, probably the code error would be defined and that would be a generic code for that special error (wich is fine)

I just say that maybe if the error can give us something like the faulting property would be awesome, but with just the fist approach will be fine... and very legible!

so don't worry about those variations! the most mportant thing is to have well defined erros!

let me know!!

[gfcittolin]

Think I got what you mean. You'd like means for knowing additional infos about what caused the error (like, in the case of area not reachable, which area is not reachable). I pĺan adding an optional and generic property info, like the one in the Node.JS API docs, so we can populate with additional infos about the error whenever they're available

[flacohenao]

OMG! that's exactly what I mean!

[flacohenao]

Uhh! I havent seen the commit yet! let me check that out! are all the errors well formated now ?

[gfcittolin]

I have done some major work on this topic, but I still want to review everything and better document it, that's why I haven't raised the hand here, just haven't had the time for that yet. But feel free to review the changes, and maybe test it out and comment about it!

[flacohenao]

Hey ghuil! any advance on this matter? should I keep testing?

[gfcittolin]

Hi Daniel, been very busy these days, sorry. I'll work on that by the end of this week

[gfcittolin]

Cleaned up and documented, good to go!

[flacohenao]

I'm already testing it! Thank you!!