search

HCA-Healthcare / elixir-mllp

An Elixir library for transporting HL7 messages via MLLP (Minimal Lower Layer Protocol)
Apache License 2.0
41 stars 19 forks source link

MLLP.Client.reconnect doesnt seem to reconnect Connection #52

Closed Fritzelblitz closed 1 year ago

Fritzelblitz commented 2 years ago

If I execute MLLP.Client.reconnect before MLLP.Client.send, we only send the request to close the TCP connection, but do not try to reopen the connection. MLLP.Client.send then reports that the connection is closed.

starbelly commented 1 year ago

@Fritzelblitz Sounds like a race between disconnect, send, and autoreconnect. I think when you say disconnect it's assumed that you don't want to reconnect, assumptions aren't great though. What we could maybe do is check if you're connected in the send and if not connect, might need an option since that would be more implicit behavior.

A PR is welcome, otherwise we'll look into this shortly.

starbelly commented 1 year ago

The more I thought about this, I'm not sure this behaviour makes sense. You tell the client to disconnect, but then you expect it to be connected on send, why not just use is_connected?/1 before you try to send?

Edit:

Ignore this, I see your issue per #51. This is a bug because we currently do a disconnect and depend on auto reconnect (if enabled) to do what you want, but as part of the disconnect process we cancel the timer for that. Even if we didn't it's still not great behavior IMO.

Instead we should attempt the cancel the reconnect timer, connect, and start the timer up again. Thanks for calling this out ❤️