Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client doesn't reconnect after EOF #147

Open
pjcdawkins opened this issue Feb 6, 2023 · 5 comments · May be fixed by #148
Open

Client doesn't reconnect after EOF #147

pjcdawkins opened this issue Feb 6, 2023 · 5 comments · May be fixed by #148

Comments

@pjcdawkins
Copy link

pjcdawkins commented Feb 6, 2023

sse/client.go

Line 217 in c6d5381

erChan <- nil

It seems on EOF, the error is ignored and the read loop silently finishes, without an error, so backoff.RetryNotify does not retry.

If this EOF check is removed (so err is returned unconditionally), the client behaves as I would expect: it reconnects after an EOF [edit: currently unsure about this].

What is the intended/expected behavior here?

@pjcdawkins pjcdawkins closed this as not planned Won't fix, can't repro, duplicate, stale Feb 6, 2023
@pjcdawkins
Copy link
Author

Sorry I was confused by the server implementation I was using... and by the default MaxElapsedTime on the backoff strategy, which causes the loop to end silently after 15 minutes, which maybe could be an issue for this library but a separate one.

@pjcdawkins
Copy link
Author

Despite other bugs distracting me, I believe this is still an issue (despite MaxElapsedTime=0), and I can reproduce it with a handwritten server which simply closes the response writer (return from the http handler).

Perhaps the library's own server implementation doesn't ever do that... I want this to work with a server that does. (Really the main thing I want is to have a client that will reconnect (after backoff) on any error, or at least one that won't stop silently.)

(Thanks for a great library)

@pjcdawkins
Copy link
Author

This is almost a duplicate of #75 depending if you treat the current behavior as a bug or a feature

@tobias-urdin
Copy link

Thanks @pjcdawkins I got stuck on this but thankfully you had already figured it out. Hoping these will get merged, pulling your PRs in manually for now.

@hspaay
Copy link

hspaay commented Jun 3, 2024

Just a note that I ran into the same issue.
If the server is restarted for whatever reason, you'd expect the client to reconnect. This isn't happening if the server stopped by closing the connection nicely as it returns an EOF.
Definitely a bug in my use-case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants