Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callback being called multiple times in GeckoPushSend.js #25

Open
johndierks opened this issue Oct 17, 2016 · 2 comments
Open

Callback being called multiple times in GeckoPushSend.js #25

johndierks opened this issue Oct 17, 2016 · 2 comments

Comments

@johndierks
Copy link

I'm running into an issue where an error state causes a callback to be called twice.

On my end, I'm sending too much data to Geckoboard list widget. Their api hangs up and causes an EPIPE error. (This part is my fault)

When this error happens the callback gets called twice on line 45 and line 55 of GeckoPushSend.js.

I'm not sure if having a callback called twice is expected behavior, and that I should account for that possibility on my end? Or if you don't think that's expected, and this is a bug, and you would like me to submit a pull request with a fix.

My opinion is that a callback should not be called twice, but I realize this is more convention than anything thing else.

Let me know, and I'm happy to do either.

@danjenkins
Copy link
Owner

wow, on end and on error events. Maybe now is the time to switch out to using request instead of using the https module directly. No, the callback shouldnt be called multiple times!

@johndierks
Copy link
Author

johndierks commented Oct 17, 2016

Great. In hunting this down, I found a couple other issues.

On error, the response from geckoboard is, in my case: {"message":"The request body is too large"}. Note that there is no success or error property.

Because response.success not defined in this case, the callback on line 43 is not reached since undefined == false evaluates to false.

Then, on the callback on line 43, GeckoBoard sends the error message on the response.message property not on response.error.

I have these fixes on a branch, I'll submit a PR. I have worked around the error callback, since in my case the error gets called back already through the end event. I'm not an expert on the HTTP module, so perhaps it needs to be accounted for there in another case. Feel free to use PR as you see fit (or not at all).

Thanks!
John

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants