Sheaffy / node-red-contrib-monzo

This is a wrapper that will allow a connection to the monzo bank API for use within node-red
MIT License
3 stars 1 forks source link

Webhook doesn't work after a deploy #5

Closed brettjenkins closed 5 years ago

brettjenkins commented 5 years ago

Hi,

Firstly great component - very cool.

Noticed no monzo messages were coming from, but the web server reverse proxy was showing the requests were coming in and were successful. Adding some console.logs in the code, and posting data to it from postman comes to the conclusion that when nodered first loads, the node works - when you deploy changes, the node stops outputting what it's getting - despite the fact the node is getting the messages.

My hunch (but I'm new to nodered dev so I could be wrong) - is that the listener on the url is hanging around, but trying to push to an old instance of the node - is it possible that we need to destroy the listener on the close function at the bottom of the webhook file, so it gets recreated with the new node var?

I've kinda come to the end of my knowledge of nodered - otherwise I'd fix it, I'm not sure how you destroy the RED.httpAdmin.post - and I might be going down the wrong path anyway.

Is it possible you can have a look?

Thanks

brettjenkins commented 5 years ago

I've fixed this after some trial and error - it was to do with the anonymous functions passed in to the callback of the post - bringing them in as named functions fixed it - I will issue a pull request soon

Sheaffy commented 5 years ago

Thanks, I will take a look at this later in the week, so long as everything works okay i will merge it.

brettjenkins commented 5 years ago

Cheers - thanks for this node-red module - it's really helpful and easy to add to :)

Sheaffy commented 5 years ago

Sorry I haven't gotten round to merging this yet, haven't had the time.. Rest assured I will get around to it!

brettjenkins commented 5 years ago

No worries - not blocking me as I'm running my patched version locally - but as I patch my local versions I'll try and contribute what I can so others can benefit :)

brettjenkins commented 5 years ago

I closed my pull request because the issue came back, and went back to the drawing board. Posted on the nodered forum about this issue - https://discourse.nodered.org/t/clearing-up-a-httpnode-post/15463 and implemented the fix suggested, which works perfectly.

Sheaffy commented 5 years ago

Closing issue as problem has been resolved thanks to @brettyj

Sheaffy commented 5 years ago

@brettyj Big thanks again, the changes are now live on NPM and version 1.2.1 can be downloaded via the pallete on node-red.

brettjenkins commented 5 years ago

No worries - what do you think about my other pull requests on here and the js library?