Skip to content

[sockjs-client] fix exponential reconnect time #1616

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

Closed
1 task done
carlosgeos opened this issue Jan 2, 2019 · 7 comments
Closed
1 task done

[sockjs-client] fix exponential reconnect time #1616

carlosgeos opened this issue Jan 2, 2019 · 7 comments

Comments

@carlosgeos
Copy link

carlosgeos commented Jan 2, 2019

  • Operating System: Debian
  • Node Version: v11.6.0
  • NPM Version: 6.5.0
  • webpack Version: 4.28.3
  • webpack-dev-server Version: 3.1.14
  • This is a bug

Code

You can see the problem here:
https://github.com/webpack/webpack-dev-server/blob/v3.1.14/client-src/default/socket.js

Expected Behavior

An exponential increase in time between reconnects with the sockjs server.

Actual Behavior

As you can see in the code I linked, the method onopen resets the retries variable to 0, so retries is always 0, and onclose always tries to reconnect waiting the minimum time. Therefore the 'infinite logspam' solution doesn't work and people experience what is depicted in #1604

Also, here (from #1604 as well):
50633098-6354a900-0f4a-11e9-95ba-22cbbc569438

How can we reproduce the behavior?

Make the sockjs server close the connection (for example because the client has a bad origin header), the client will try to reconnect forever without respecting the intended timeout.

It is obvious looking at the code anyway.

PR

I've already submitted a PR.

@alexander-akait
Copy link
Member

@carlosgeos thanks for issue

carlosgeos pushed a commit to carlosgeos/webpack-dev-server that referenced this issue Jan 31, 2019
carlosgeos pushed a commit to carlosgeos/webpack-dev-server that referenced this issue Feb 26, 2019
carlosgeos pushed a commit to carlosgeos/webpack-dev-server that referenced this issue Feb 26, 2019
carlosgeos pushed a commit to carlosgeos/webpack-dev-server that referenced this issue Feb 26, 2019
carlosgeos pushed a commit to carlosgeos/webpack-dev-server that referenced this issue Feb 26, 2019
carlosgeos pushed a commit to carlosgeos/webpack-dev-server that referenced this issue Feb 26, 2019
@alexander-akait
Copy link
Member

Should be fixed in v4

@amakhrov
Copy link

I don't see any related changes in the code, and it seems that v4 still keeps retrying indefinitely without any exponential backoff.
@alexander-akait could you please clarify?

@alexander-akait
Copy link
Member

@amakhrov Can you check you have a latest version? If you still have a problem, please provide example how we can reproduce it

@amakhrov
Copy link

We're on 4.7.3

The exact scenario is webpack-dev-server terminating the socket, if it didn't receive the "pong" from the client as a part of heartbeat check.

@alexander-akait
Copy link
Member

@amakhrov Maybe you can provide example? Also you can send a PR without tests (if you have solution), so I will look at code and try to search potential problem places

@amakhrov
Copy link

Now as I think further about the use case, I understand why retries do not work properly.

In this case a client socket successfully connects, but then the server terminates it (due to the lack of ping response). Every time a socket successfully connects, retries counter is reset - which is totally valid in other scenarios, but effectively disables the exponential retries logic for my case.

I'm not sure it's possible for the client socket to differentiate between the two cases, though

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

Successfully merging a pull request may close this issue.

3 participants