sonntam / node-red-contrib-xstate-machine

A xstate-based state machine implementation using state-machine-cat visualization for node red.
MIT License
22 stars 8 forks source link

xstate node irrecoverably crashes node red if exception in node occurs #106

Closed Kilill closed 7 months ago

Kilill commented 7 months ago

had an issue where an action was misspelled/missing which resulted in the node throwing an exception and permanently crashing Node Red ie. Node Red could no longer be restarted since it crashes again on the same issue

Had to manually edit the flows.json file and remove the xstate entry.

I don't think a misconfigured node should be able to crash Node Red that hard...

Node-Red running as a service on ubuntu 22.04:

nov. 29 17:36:54 cyclops Node-RED[2576060]: 29 Nov 17:36:54 - [info] Node-RED version: v3.1.0
nov. 29 17:36:54 cyclops Node-RED[2576060]: 29 Nov 17:36:54 - [info] Node.js  version: v20.9.0
nov. 29 17:36:54 cyclops Node-RED[2576060]: 29 Nov 17:36:54 - [info] Linux 5.15.0-88-generic x64 LE

How to reproduce:

Observ that this will most proably crash NodeRed so make backup of .node-red/projects/Home/flows.json

  1. create a new flow tab
  2. drop an smxstate node into it
  3. edit the properties of the node and rename one of the actions (incrementCounter or resetCounter)
  4. save
  5. deploy
  6. observ that node red is now dead and can not be restarted

Log output from node red:

nov. 29 17:36:41 cyclops Node-RED[2575948]: 29 Nov 17:36:41 - [info] Started flows
nov. 29 17:36:41 cyclops Node-RED[2575948]: 29 Nov 17:36:41 - [red] Uncaught Exception:
nov. 29 17:36:41 cyclops Node-RED[2575948]: 29 Nov 17:36:41 - [error] ReferenceError: incrementCounter is not defined
nov. 29 17:36:41 cyclops Node-RED[2575948]:     at SMXSTATE node:def654576503dc8a:126:16
nov. 29 17:36:41 cyclops Node-RED[2575948]:     at SMXSTATE node:def654576503dc8a:135:3
nov. 29 17:36:41 cyclops Node-RED[2575948]:     at Script.runInContext (node:vm:134:12)
nov. 29 17:36:41 cyclops Node-RED[2575948]:     at new StateMachineNode (/home/node-red/.node-red/node_modules/node-red-contrib-xstate-machine/dist/smxstate-node.js:435:34)
nov. 29 17:36:41 cyclops Node-RED[2575948]:     at Object.createNode (/usr/lib/node_modules/node-red/node_modules/@node-red/runtime/lib/flows/util.js:157:27)
nov. 29 17:36:41 cyclops Node-RED[2575948]:     at Flow.start (/usr/lib/node_modules/node-red/node_modules/@node-red/runtime/lib/flows/Flow.js:260:54)
nov. 29 17:36:41 cyclops Node-RED[2575948]:     at async Object.start [as startFlows] (/usr/lib/node_modules/node-red/node_modules/@node-red/runtime/lib/flows/index.js:398:17)
nov. 29 17:36:41 cyclops systemd[1]: nodered.service: Main process exited, code=exited, status=1/FAILURE
nov. 29 17:36:41 cyclops systemd[1]: nodered.service: Failed with result 'exit-code'.
sonntam commented 7 months ago

Thank you for taking the time to file a bug report and the steps to reproduce it. That is very much appreciated!

I don't think a misconfigured node should be able to crash Node Red that hard...

You are absolutely right - this will have top priority next time I have free time to work on the node.

I have one handy hint for this case when a node misbehaves. Just set the environment variable NODE_RED_ENABLE_SAFE_MODE=true before launching node red. This will allow you to load the node-red editor and fix/change your flows but it won't start the flows until you hit the deploy button. This way you don't have to edit your flows.json manually.

Edit: If you think you are up for it you could make a proposal on how to fix this problem. Then PRs are most welcome šŸ˜‰

Kilill commented 7 months ago

Edit: If you think you are up for it you could make a proposal on how to fix this problem. Then PRs are most welcome šŸ˜‰

I would if i was a lot more fluent in node and js :-) and how to build a node... Did have a look though and as far as my limited understanding goes it seems that the promise on line 435 of smxstate-node.js needs to be wrapped somehow since afaik exceptions are not propagated properly out of an async/promise ...

sonntam commented 7 months ago

It was missing a simple .catch() in the promise execution chain. It should be fixed in Release V1.3.3!