facebook / create-react-app

Set up a modern web app by running one command.
https://create-react-app.dev
MIT License
102.77k stars 26.87k forks source link

Possible fix for webpackHotDevClient disconnecting from server. #8091

Open gamedev8 opened 4 years ago

gamedev8 commented 4 years ago

Hello I would like to submit a fix for the issue people are having with the websocket in webpackHotDevClient.js disconnecting from the server. The websocket is timing out after some time, so we simply ping the server periodically to prevent it from timing out. Here is the code I've been using.

// Ping the server to prevent the client from disconnecting
var ping = function() {
  if (connection && (connection.readyState === connection.OPEN)) {
    connection.send("ping");
    setTimeout(ping, 3000);
  }
};

connection.onopen = function() {
  if (typeof console !== 'undefined' && typeof console.info === 'function') {
    console.info(
      'The development server has connected.'
    );
  }

  ping();
};

I have a branch ready here. I can submit a PR. https://github.com/vibelife/create-react-app/blob/master/packages/react-dev-utils/webpackHotDevClient.js#L85

ianschmitz commented 4 years ago

Hi @gamedev8. You say others were experiencing issues? Would you mind providing a link for reference?

drather19 commented 4 years ago

I am experiencing the same issue. If I don't make any code edits, the dev server web socket connection times out after 5 minutes. Is this related to the switch to native ws transport from sockjs done in https://github.com/facebook/create-react-app/pull/7988?

silverwind commented 4 years ago

Every websocket connection should have a keepalive to prevent firewalls from terminating the connection unexpectedly because they think it's "idle". Sockjs defaults it to one ping from server to client every 25s which seems like a good default to me.

ianschmitz commented 4 years ago

It would seem that a fix is more appropriate in webpack-dev-server if that's the current behavior of sockjs which webpack uses by default (and is transitioning to WS default in next major).

sockjs-node code for reference: https://github.com/sockjs/sockjs-node/blob/4c9f3009be13962b0785138ccdf527c5abe925f6/lib/transport/websocket.js#L74-L82

drather19 commented 4 years ago

That sounds reasonable regarding where the fix should go, though in the meantime, is the intent to stick with the current settings in CRA and wait out a change in webpack-dev-server?

Love some of the updates in CRA 3.3.0, but am resorting to manually patching the dev server locally to get around the disconnects (which does work for me) rather than pinning back.

ianschmitz commented 4 years ago

We upgraded to the WS implementation to fix some other issues, so i doubt we'll be going back to the legacy sockjs implementation as WS is the path forward. If webpack-dev-server isn't responsive we can potentially patch things up in CRA to help.

I created an upstream issue at https://github.com/webpack/webpack-dev-server/issues/2403.

gamedev8 commented 4 years ago

Hey @ianschmitz There is an important flaw with the code I posted and any kind of heartbeat implementation - When you are stepping through your code the browser is tying up your whole application, so the client and server will not be able to communicate... and eventually time out. So IMHO a better solution is for the server to somehow not timeout the client, or for the client to automatically reconnect.

ianschmitz commented 4 years ago

Agreed. I'm hopeful we can get this fixed upstream.

ianschmitz commented 4 years ago

Looks like the server side socket logic has been improved in WDS. Would be great if someone can test their fix to see if it closes this issue.

gamedev8 commented 4 years ago

The changes are not working for me. It still times out after 60 seconds. I upgraded my webpack-dev-server to version 3.10.2 Is there anything else that I need to do?

ianschmitz commented 4 years ago

We pin the version of webpack-dev-server, so you've likely just installed another version in your tree that isn't being used. You can try yarn resolutions.

gamedev8 commented 4 years ago

I had deleted the webpack-dev-server version I had from my node_modules folder, then installed 3.10.2. When I tried to run my application CRA detected the new version and printed a long warning. I had to put SKIP_PREFLIGHT_CHECK=true in my .env file to get around the warning.

ianschmitz commented 4 years ago

Yeah that's not gonna work. Try npm ls webpack-dev-server. You'll likely see two versions installed. You'll need to use yarn resolutions to force the newer version to be installed in the correct location to test the fix.