search

segmentio / nsq.js

NSQ client for nodejs
203 stars 34 forks source link

conn: ignore all writes after the socket is closed #60

Closed yields closed 9 years ago

yields commented 9 years ago

this should fix the write after end errors we are seeing.

cc @gjohnson

juliangruber commented 9 years ago

is that in case of a reconnect in-progress? my initial thought is that i would want it to throw if something bad happened, or have a strategy for queuing up writes until the connection is up again

gjohnson commented 9 years ago

@juliangruber we're getting this noise when shutting down the process (after calling close()).

Error: 127.0.0.1:5001: write after end
    at writeAfterEnd (_stream_writable.js:132:12)
    at Socket.Writable.write (_stream_writable.js:180:5)
    at Socket.write (net.js:615:40)
    at Connection.command (/.../node_modules/nsq.js/lib/connection.js:177:8)
    at Connection.finish (/.../node_modules/nsq.js/lib/connection.js:343:8)
    at Message.finish (/.../node_modules/nsq.js/lib/message.js:49:13)
yields commented 9 years ago

i might be wrong but i think the socket gets reused? so conn.end() is never called?

yields commented 9 years ago

ah yeah, reconnect() never calls conn.end() so the socket gets re-used there so i think we're good? i think the Writer calls conn.end() when you call writer.close(), so that's why we never see write after end on reconnections.

juliangruber commented 9 years ago

aaaah gotcha, i thought this was ignoring writes to a closed socket but if a connection is closed i'm all +1