search

markabrahams / node-net-snmp

JavaScript implementation of the Simple Network Management Protocol (SNMP)
209 stars 97 forks source link

writeInt32 Error / Error Response Not Generated #214

Open morrisrob opened 1 year ago

morrisrob commented 1 year ago

Hello, I recently updated to 3.8.4 (and now 3.9.0). I'm working on some testing and I have run into an issue which looks like it is most likely related to the recent work on integer range checking, etc.

Prior to these changes, if I tried writing a non-integer value, it looks like it would get rounded and still send the value. After these changes, an error is getting generated in the library (which is fine / expected), but no response is getting sent (neither an error response or a rounded value).

I do have my request handler wrapped in a try catch, but it looks like this doesn't help in this situation, since we're already at the point where it is trying to get the response ready and send it out.

I would think that one of 2 things should happen in this case:

1) Send an error response -or- 1) Generate an error in the library so that the error can be identified in application logs, but still send along a rounded value to mimic what was done in the past)

I don't necessarily have a preference on either option. Attached is a screenshot of the call stack of this error. Thank you.

image

markabrahams commented 1 year ago

Hi @morrisrob - thanks for logging this.

In this case, I'm more inclined to focus on stopping illegal data from entering the MIB - I don't see a good reason to tolerate such antisocial behaviour! :-)

So the two solutions to this end are: (1) Don't put illegal data in the MIB - it's completely under the caller's control, so they can validate first to avoid this, or (2) Better, the MIB provider definitions have typing information for scalars and table columns. The library could - and I think should - validate any MIB set/add API call data against the MIB provider types, and fail non-conforming calls.

Thoughts?

morrisrob commented 1 year ago

@markabrahams Thank you for the response on this. Yes, I agree that this is completely within the caller's control and that the caller should be validating the data first to ensure that it contains legal data. My opening of this issue was based on the fact that the behavior was changed somewhere along the way and that I was surprised that an error was getting generated in the application but it just ended up as a timed out SNMP request.

I've already fixed this on my end by adding some validation and corrected the spot where I was adding an incorrect illegal data to the MIB.

I agree that #2 that you mentioned would be a good solution. Not sure how other users would want to see these types of situations handled, so perhaps adding this as some sort of configurable option or exposing this in a way that it could be caught / handled by the request handler if the user wants to do that?