node-red / node-red

Low-code programming for event-driven applications
http://nodered.org
Apache License 2.0
19.51k stars 3.37k forks source link

Uncaught Exception while stopping flows (during deploy) #4493

Closed bombjackit closed 8 months ago

bombjackit commented 9 months ago

Current Behavior

Node-red crashed with this dump (while deploying)

20 Dec 16:57:05 - [red] Uncaught Exception: 20 Dec 16:57:05 - [error] ConnectionError: Cannot close a pool while it is connecting at ConnectionPool._close (S:.node-red\node_modules\mssql\lib\base\connection-pool.js:538:30) at S:.node-red\node_modules\mssql\lib\base\connection-pool.js:521:12 at new Promise () at ConnectionPool.close (S:.node-red\node_modules\mssql\lib\base\connection-pool.js:520:12) at node.connectionCleanup (S:.node-red\node_modules\node-red-contrib-mssql-plus\src\mssql.js:335:42) at node.disconnect (S:.node-red\node_modules\node-red-contrib-mssql-plus\src\mssql.js:459:22) at mssql. (S:.node-red\node_modules\node-red-contrib-mssql-plus\src\mssql.js:834:21) at Node.close (S:\AppData\Roaming\npm\node_modules\node-red\node_modules\@node-red\runtime\lib\nodes\Node.js:340:26) at stopNode (S:\AppData\Roaming\npm\node_modules\node-red\node_modules\@node-red\runtime\lib\flows\Flow.js:745:31) at Flow.stop (S:\AppData\Roaming\npm\node_modules\node-red\node_modules\@node-red\runtime\lib\flows\Flow.js:387:35)

Expected Behavior

Node-red must not crash for this unacaught exception, as I see at this line (see link below) there is a try catch

https://github.com/node-red/node-red/blob/0e8d3127944a91f39fb49bcb4b388b184ce11edd/packages/node_modules/%40node-red/runtime/lib/flows/Flow.js#L388C39-L388C39

Steps To Reproduce

In my nodered I had a flow that 1 time a second do an update to some tables in sql server (on localhost), maybe this cause the exception (Cannot close a pool while it is connecting) but the problem is that node-red does must not crash with an exception, only report.

Environment

hardillb commented 9 months ago

Node-RED must exit on an uncaught exception, because we have no way of knowing what state it has left any 3rd party node. It is up to the authors of those 3rd party nodes to catch all exceptions from their code (or dependencies).

This is an issue with the node-red-contrib-mssql-plus and should be raised with them

bombjackit commented 9 months ago

Yes, going deeper there was a problem in node-red-contrib-mssql-plus (see https://github.com/bestlong/node-red-contrib-mssql-plus/issues/91)

Steve-Mcl commented 8 months ago

closing as issue is with contrib node.