markabrahams / node-net-snmp

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

Add delete add failure #160

Closed derrell closed 3 years ago

derrell commented 3 years ago

Using RowStatus to add and delete rows, in some scenarios, add, delete, add fails. I have provided here a demo that reproduces the problem.

The final add will fail. It's failing because tryCreateInstance created the row, but this.mib.lookup (oid); at the end of tryCreateInstance then fails (quite unexpectedly).

derrell commented 3 years ago

I've greatly simplified the test. Notice that there are now console.log calls showing "Row", after adding a new row. The first one has the correct row values; the second one yields null. No manager requests are required any longer to see the problem; just run the test application.

derrell commented 3 years ago

It almost looks like pruneUpwards is pruning one too far. There are no longer any table entries (rows) after the delete, but something isn't left in the same state as before the add...

derrell commented 3 years ago

My supposition has been somewhat confirmed by commenting out instanceParentNode.pruneUpwards(); in deleteTableRow. With that commented out, the second addTableRow works as it should. (I'm sure leaving it commented out would leave the tree in an incorrect state, but it does seem to show that something wrong is going on with the pruning.)

derrell commented 3 years ago

Ok, getting someplace. I've added mib.dump() calls, with leavesOnly: false, to see what's going on with the tree. The delete has done something that has caused, on the second add, the Table entry not to be there, and the table's two columns to have been added with incorrect OIDs and not members of the Table.

markabrahams commented 3 years ago

Hi @derrell - good spot there! The upwards tree prune was working fine (although not pruning did - as you pointed out - workaround the problem), but the agent's mib was hanging onto the state of the now deleted provider tree node in its mib.providerNodes list. So I needed to clear that entry when the last row of a table was deleted.

An an aside, the mib.dump() options weren't working properly, so I've fixed those as well. The leavesOnly: false option should (and does now) dump every node from root down, including all internal nodes.

Pushed the fix for this to master and published in version 3.2.2 of the npm.