search

uber-archive / node-statsd-client

Node.js client for statsd
ISC License
28 stars 9 forks source link

Time-based queue flushing is broken after socket is closed #31

Closed mpareja closed 8 years ago

mpareja commented 8 years ago

Here is a failing test case: https://github.com/mpareja/node-statsd-client/blob/queue-flush/test/socket.js#L214.

The internal tear-down of the socket performed by EphemeralSocket calls the public EphemeralSocket.close method. This results in the destruction of the PacketQueue instance which permanently clears the time-based queue flushing. Time based queue flushing is never re-enabled.

Raynos commented 8 years ago

That sounds like a bug to me.

Would you like to open a PR with a fix ?

mpareja commented 8 years ago

@Raynos Yeah, I'll submit a PR.

mpareja commented 8 years ago

FYI I noticed the DNS resolver would also stop responding with DNS query results after socket timeout. That's been fixed by my PR as well.

Is there anything else that needs to happen for a new version of this package to be published?

niclic commented 8 years ago

+1

Raynos commented 8 years ago

Merged & published 1.5.0

mpareja commented 8 years ago

Sweet, thanks @Raynos. :+1: