Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix issue: reconnection only happends for 1 time after connection drops #136

Merged
merged 1 commit into from
Jul 7, 2020

Conversation

icy-fish
Copy link
Contributor

Since the PR #125 fixed duplicate connections after reconnection by using a connectionInProgress lock to avoid function connect() be called duplicately.

But it forgot to release the connectionInProgress lock when request error happends, in that case, our client can only retry for 1 time and never get the lock again.

So it's needed to release the connectionInProgress lock when error happends.

Copy link
Contributor

@aslakhellesoy aslakhellesoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! 🙏

test/eventsource_test.js Outdated Show resolved Hide resolved
Since the PR EventSource#125 fixed duplicate connections after reconnection by using a `connectionInProgress` lock to avoid function `connect()` be called duplicately.

But it forgot to release the `connectionInProgress` lock when request error happends, in that case, our client can only retry for 1 time and never get the lock again.

So it's needed to release the `connectionInProgress` lock when error happends.

Signed-off-by: icy_fish <keith519@qq.com>
@hn3000
Copy link

hn3000 commented Jul 7, 2020

Hi,

any idea when this will be merged and published?

Is there any way we could help?

Thanks!

@joeybaker
Copy link
Contributor

Hi! I'm happy to merge, but I don't have npm publish access so will have to wait for someone who does

@joeybaker joeybaker merged commit 46fe04e into EventSource:master Jul 7, 2020
@aslakhellesoy
Copy link
Contributor

@joeybaker can you update History.md please? Then I’ll make a release.

@mythz
Copy link

mythz commented Mar 16, 2021

Can we get this released please? the current release on npm is still broken.

joeybaker added a commit that referenced this pull request Mar 16, 2021
@joeybaker
Copy link
Contributor

@aslakhellesoy #146

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

Successfully merging this pull request may close these issues.

5 participants