markabrahams / node-net-snmp

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

Unhandled error in Listener.processIncoming crashing node #213

Closed morrisrob closed 1 year ago

morrisrob commented 1 year ago

Version 3.8.4 Upgraded from 3.6.3

After upgrading to 3.8.4, I'm finding that there is an unhandled error that is crashing my node instance when receiving 'empty' requests. It looks like these 'bad' requests are correctly throwing a 'TypeError: Value read as integer null is not an integer' in readInt32 but then these are not getting caught / handled anywhere. Unfortunately these empty requests are out of our control, as they are getting generated from some sort of network monitoring utility/tool on the same network.

Below is a screenshot of buffer and rinfo logged on line 4539 (Agent.protype.onMsg) to show the type of requests that we are receiving & causing this issue.

Am I missing a way to catch / handle these errors? Otherwise, should the var message = Listener.processIncoming (buffer, this.authorizer, this.callback); in Agent.prototype.onMsg get wrapped in a try/catch, similar to Session.prototype.onMsg?

image

morrisrob commented 1 year ago

Well, after downgrading back to 3.6.4, I'm experiencing similar issues, although the unhandled error is generated from a different spot. So looks like not specifically related to / caused by my upgrade to 3.8.4, but rather an environmental change on my end. Here is a screenshot of where the error is getting thrown from when on 3.6.4.

So either way, looks like catching / handling the error in Agent.prototype.onMsg or somewhere else could fix this.

image

markabrahams commented 1 year ago

Hi @morrisrob - yes, that's a good suggestion - thanks for reporting this.

As you suggest, I've wrapped the agent's and receiver's message processing call in a try/catch. If the message processing throws an error, it will send a new ProcessingError error class to the callback, which contains these fields:

So the caller can now do what they want with this (including not crash!)

The fix is on master now and published in version 3.9.0 of the npm.

morrisrob commented 1 year ago

@markabrahams thanks for the quick response & fix!