search

zwave-js / node-red-contrib-zwave-js

The most powerful, high performing and highly polished Z-Wave node for Node-RED based on Z-Wave JS. If you want a fully featured Z-Wave framework in your Node-RED instance, you have found it.
MIT License
47 stars 6 forks source link

Unhandled promise rejection in UI when controller returns error #30

Closed marcus-j-davies closed 3 years ago

marcus-j-davies commented 3 years ago

@hufftheweevil ,

Seeing to a bug report (hopefully now fixed in 3.0). in doing so, come across a bug in the UI.

This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason:
Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at ServerResponse.setHeader (_http_outgoing.js:558:11)
    at ServerResponse.header (C:\Users\Marcus Davies\Desktop\New folder\node_modules\express\lib\response.js:771:10)
    at ServerResponse.contentType (C:\Users\Marcus Davies\Desktop\New folder\node_modules\express\lib\response.js:599:15)
    at ServerResponse.send (C:\Users\Marcus Davies\Desktop\New folder\node_modules\express\lib\response.js:145:14)
    at C:\Users\Marcus Davies\.node-red\node-red-contrib-zwave-js\zwave-js\ui\server.js:56:39
    at Object.Input [as request] (C:\Users\Marcus Davies\.node-red\node-red-contrib-zwave-js\zwave-js\zwave-js.js:255:21)

This kills the node process, and occurs when the zwave driver returns an exception, (this is handled in the main module), as the error is bubbled up to the user when using a node message, as apposed to the UI.

I tried removing a failed node (that was responding to pings) so ZWJS refused, and threw.

EDIT: On the dev branch

https://github.com/zwave-js/node-red-contrib-zwave-js/blob/0c988919d01686aacb500e57005cca3f02e21436/zwave-js/ui/server.js#L56

marcus-j-davies commented 3 years ago

Think I have sorted it.

The main problem I believe, was the timeout not being cancelled (Cannot set headers after they are sent to the client) This now happens, and I also handle error on the $.ajax call, to communicate any error. I have set noWait to fasle on the removeFailedNode command - so errors can be communicated

See this commit for the changes, if you think this can be done better, please do change this approach https://github.com/zwave-js/node-red-contrib-zwave-js/commit/505cec6a05eae08d580b142d47ce54a37e0aa47a

EDIT: After seeing how you do the same thing, have matched it. so the fix, is just to cancel the timer, and manage the response for RemoveFailedNode https://github.com/zwave-js/node-red-contrib-zwave-js/commit/4ad5c901b51fb424e4d571e49843a3c404f21fbc

hufftheweevil commented 3 years ago

Sorry, I haven't had as much free time lately. Looks like you got the right idea there. Did that fix it?

hufftheweevil commented 3 years ago

Whoops - wrong button...

marcus-j-davies commented 3 years ago

it did - all working.

No worries.