Fix data parameter always undefined on callback for track method #231
Conversation
|
@danieljackins I didn't need to change anywhere else since the place I added solved my problem. Feel free to add in the other places. |
|
@danieljackins just added the callback parameter to the |
| return | ||
| } | ||
|
|
||
| if (this.queue.length >= this.flushAt) { | ||
| this.flush() | ||
| this.flush(callback) | ||
| } | ||
|
|
||
| if (this.flushInterval && !this.timer) { |
| return | ||
| } | ||
|
|
||
| if (this.queue.length >= this.flushAt) { | ||
| this.flush() | ||
| this.flush(callback) |
There was a problem hiding this comment.
Weird bug, may not cause issues, but this actually causes the callback to be called twice now as its the exact same callback pulled from the item when flushed.
https://github.com/segmentio/analytics-node/blob/master/index.js#L247-L259
There was a problem hiding this comment.
Yes, the callback is running twice for me as well. Is there a plan to fix that? Or should I log a new issue?
There was a problem hiding this comment.
Hi. Was the fix for this released with v5.1.0?
There was a problem hiding this comment.
@pooyaj It's fixed locally. I'm writing some tests and I'll create a PR for it.
There was a problem hiding this comment.
@BrandonNoad Can you give me a minimal working example that illustrates the issue?
For issue #1: seems unlikely given they're only called when the request to segment servers actually finishes(successfully or not).
As for issue #2: I'll have to see some code.
There was a problem hiding this comment.
@BrandonNoad Please open a new issue for it though.
There was a problem hiding this comment.
@pbassut Yes, I can open a new issue with some code to reproduce the behaviour.
For issue #1: seems unlikely given they're only called when the request to segment servers actually finishes(successfully or not).
I may have been incorrect in saying "too early". That was just an assumption based on the fact that the callback is being called twice and in the first call, batch is undefined.
Fixes: #230