search

sbt / sbt-remote-control

Create and manage sbt process using unicorns and forks
Other
74 stars 14 forks source link

Report connection errors and retry connecting #81

Closed havocp closed 10 years ago

havocp commented 10 years ago

Recommend looking at the "files changed" not commit-by-commit, I took a circuitous route to arrive.

jsuereth commented 10 years ago

Is there a reason that opening the connection is explicit?

I'm not a huge fan of violating RAII (resource acquisition is initialization) if we can help it....

havocp commented 10 years ago

Because if opening is a side effect of adding the onConnect callback, you'd have to always do onError first and then onConnect or you might miss the error, which would be ... surprising.

The alternative design I think would be to remove onConnect and onError and make open() take those two callbacks as parameters. Then it avoids ordering concerns because you add both callbacks at once. If multiple people open() then they would each be guaranteed to get at least one of their callbacks called, even if the connection was already open or already closed.

That's probably better; I can do that. Thoughts?

jsuereth commented 10 years ago

Yeah, you should pass both handlers when attempting to connect, OR the onConnect handler passes you a Try[_] or some other mechanism of handling the failure directly...

havocp commented 10 years ago

how about this?

jsuereth commented 10 years ago

LGTM!

jsuereth commented 10 years ago

cc @skyluc I think this will affect your client....