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

server.close no longer works as of 3.4.0 #1990

Closed
1 task
Aghassi opened this issue Jun 7, 2019 · 8 comments · Fixed by #2001
Closed
1 task

server.close no longer works as of 3.4.0 #1990

Aghassi opened this issue Jun 7, 2019 · 8 comments · Fixed by #2001

Comments

@Aghassi
Copy link

Aghassi commented Jun 7, 2019

  • Operating System: macOS 10.14.4
  • Node Version: 10.16.0
  • NPM Version: 6.9.0
  • webpack Version: 4.33.0
  • webpack-dev-server Version: 3.6.0
  • [ x ] This is a bug
  • This is a modification request

Code

We have a block of code where we run webpack-dev-server and then kick off Cypress and Lighthouse. In each of these blocks of code our webpack builds fine, and the server runs. However, we can no longer kill the server with server.close. It does nothing (seemingly) and just leaves the process open and running.

// webpack.config.js
      new Promise((resolve, reject) => {
        server.listen(port, '0.0.0.0', async () => {
          try {
            const results = await (open ? cypress.open(cypressConfig) : cypress.run(cypressConfig));
            // Fail if there are tests that failed
            if (results.totalFailed > 0) {
              throw new CLIError(
                `Some tests failed. See results for more details ${emoji.get('point_up')}`,
              );
            }
            resolve(results);
          } catch (err) {
            // Reject on any other failures
            reject(err);
          } finally {
            console.log('server :', server);
            // Always close the server
            server.close();
          }
        });
      });

Expected Behavior

The server should close when I call server.close

Actual Behavior

The process remains open (we know this because our integration tests are failing because they hang on CI)

For Bugs; How can we reproduce the behavior?

Use the node api to start the server and then call close on it. I can't provide a good example right now since this is code internal to the company I work for. From what I can tell though in debugging:

https://github.com/webpack/webpack-dev-server/blob/master/lib/servers/SockJSServer.js#L57

The above line doesn't work because the instance it is calling close on doesn't expose a close function. Below is the object I get back if I log out what connection is.

[ SockJSConnection {
    _session:
     Session {
       session_id: undefined,
       heartbeat_delay: 25000,
       disconnect_delay: 5000,
       prefix: '/sockjs-node',
       send_buffer: [],
       is_closing: false,
       readyState: 1,
       timeout_cb: [Function],
       to_tref:
        Timeout {
          _called: false,
          _idleTimeout: 25000,
          _idlePrev: [TimersList],
          _idleNext: [TimersList],
          _idleStart: 37401,
          _onTimeout: [Function],
          _timerArgs: undefined,
          _repeat: null,
          _destroyed: false,
          [Symbol(unrefed)]: false,
          [Symbol(asyncId)]: 21090,
          [Symbol(triggerId)]: 21084 },
       connection: [Circular],
       emit_open: null,
       recv: [WebSocketReceiver] },
    id: '5af72c23-aeb9-45bd-a321-0577443d1ea5',
    headers:
     { host: 'localhost:34212',
       'user-agent':
        'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_5) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.1.1 Safari/605.1.15',
       origin: 'http://localhost:34212' },
    prefix: '/sockjs-node',
    remoteAddress: '127.0.0.1',
    remotePort: 57216,
    address: { address: '127.0.0.1', family: 'IPv4', port: 34212 },
    url: '/sockjs-node/261/ciymzuaz/websocket',
    pathname: '/sockjs-node/261/ciymzuaz/websocket',
    protocol: 'websocket',
    _events: [Object: null prototype] { close: [Function] },
    _eventsCount: 1 } ]

The above is called via

https://github.com/webpack/webpack-dev-server/blob/master/lib/Server.js#L765

Which prior to 3.4.0, was an empty array in our case.

What I'm trying to figure out is how the change to the internal SockJS server setup is effecting this.

For Features; What is the motivation and/or use-case for the feature?

@alexander-akait
Copy link
Member

Can't reproduce please provide example

@ruijieshi
Copy link

Hi,

A follow up on here:

We want to control what error to display to user if Webpack compilation fails in cypress command. So if webpack fails, we want to close server and exit without the Webpack long stack error message.

As below, we have a class extends from @oclif/command which this.exit should end the process

compiler.hooks.done.tap('done', (stats) => {
   const fail = // Code to get if Webpack fails;
   if(fail) {
           server.close();
           this.log(errorMessage we decide to show);
           this.exit(1);
  }
}

But with this.exit(1), it still shows a long error message from Webpack after our custom log. Only process.exit(1) will block the error. But it will also stop our integration test. Do you have any suggestion to close the server with out showing the long error message from Webpack?

Thanks!

@alexander-akait
Copy link
Member

@ruijieshi I can't understand you. It is related to this issue? Please provide more information (better reproducible test repo)

@alexander-akait
Copy link
Member

alexander-akait commented Jun 7, 2019

/cc @Aghassi Can you provide more information? Maybe problem in code? We want to freeze currently master branch and start working on next major release, so if you want to fix it asap, please provide more context and information (better simple example), thanks

@alexander-akait
Copy link
Member

@ruijieshi using compiler.hooks.done.tap for catching error from webpack-dev-server is unsafe, we return server and server have error callback, please use error callback for catching errors

@Aghassi
Copy link
Author

Aghassi commented Jun 7, 2019

@evilebottnawi I will get a demo repo together. I have not had a chance today to do so. Many meetings.

@krohrsb
Copy link

krohrsb commented Jun 7, 2019

I was able to reproduce the odd behavior. See code below. The key thing is the behavior of open.

On 3.3.1 and 3.3.0 (at least) open: true doesn't actually open anything for me. This allowed server.close to function because as of 3.4.0 open: true does open a browser for me, and this blocks the server.close functionality. Expected? You tell me :)

Version Open Flag Behavior
3.3.0 true Browser does not open (not expected), but server.close works (process exits)
3.3.0 false Browser does not open (expected), and server.close works (process exits)
3.3.1 true Browser does not open (not expected), but server.close works (process exits)
3.3.1 false Browser does not open (expected), and server.close works (process exits)
3.4.0 true Browser does open (expected), but server.close does not work (process hangs)
3.4.0 false Browser does not open (expected), and server.close works (process exits)

🗒 Latest (3.7.1) has the same behavior as 3.4.0

const WebpackDevServer = require('webpack-dev-server');
const webpack = require('webpack');
const compiler = webpack({
  module: {
    rules: []
  },
  plugins: []
});

const server = new WebpackDevServer(compiler, {
  open: true, // fails on 3.4.0, works on < 3.4.0
  open: false // works on 3.4.0
});

(new Promise((resolve, reject) => {
  server.listen(3000, '0.0.0.0', () => {
    console.log('waiting 3s then close server and resolve');
    setTimeout(() => {
      server.close(() => resolve({
        success: true
      }));
    }, 10000)


  });
})).then((...args) => console.log('done', args));

@hiroppy
Copy link
Member

hiroppy commented Jun 8, 2019

@krohrsb Thank you for submitting the code. I created a pr to fix this issue. #2001

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.

5 participants