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

CTRL + C not killing webpack dev server in Windows cousing port 8080 to be locked #2562

Closed
McSneaky opened this issue May 6, 2021 · 5 comments · Fixed by adonisjs/assembler#26
Assignees
Labels
Type: Question Needs clarification

Comments

@McSneaky
Copy link
Contributor

McSneaky commented May 6, 2021

Package version

AdonisJS V5

package.json
{
  "name": "test",
  "version": "1.0.0",
  "private": true,
  "scripts": {
    "build": "node ace build --production",
    "start": "node server.js",
    "dev": "node ace serve --watch",
    "lint": "eslint . --ext=.ts"
  },
  "devDependencies": {
    "@adonisjs/assembler": "^5.1.1",
    "@symfony/webpack-encore": "^1.2.0",
    "adonis-preset-ts": "^2.1.0",
    "eslint": "^7.25.0",
    "eslint-plugin-adonis": "^1.3.1",
    "pino-pretty": "^4.7.1",
    "typescript": "^4.2.4",
    "youch": "^2.2.2",
    "youch-terminal": "^1.1.1"
  },
  "dependencies": {
    "@adonisjs/core": "^5.1.6",
    "@adonisjs/repl": "^3.1.2",
    "@adonisjs/session": "^6.0.3",
    "@adonisjs/shield": "^7.0.2",
    "@adonisjs/view": "^6.0.1",
    "proxy-addr": "^2.0.6",
    "reflect-metadata": "^0.1.13",
    "source-map-support": "^0.5.19"
  }
}

Node.js and npm version

node -v
v14.16.1
npm --version
6.14.12

Sample Code (to reproduce the issue)

  • Create new project with Webpack
  • Start server with node ace serve --watch
  • Kill server with CTRL + C
  • Start server again with node ace serve --watch
  • Look at error :)
Project setup
PS C:\Users\McSnack\Projects> npm init adonis-ts-app test
npx: installed 191 in 32.135s
    _       _             _         _
   / \   __| | ___  _ __ (_)___    | |___
 / ___ \ (_| | (_) | | | | \__ \ |_| \__ \
/_/   \_\__,_|\___/|_| |_|_|___/\___/|___/


CUSTOMIZE PROJECT
❯ Select the project structure · web
❯ Enter the project name · test
❯ Setup eslint? (y/N) · true
❯ Setup prettier? (y/N) · false
❯ Configure webpack encore for compiling frontend assets? (y/N) · true

RUNNING TASKS
❯ Scaffold project 155 ms
❯ Install dependencies 1.58 min
❯ Configure installed packages 1.55 s
❯ Configure webpack encore 2.13 min

[ success ]  Project created successfully
Outcome of starting server, closing it and starting it again
PS C:\Users\McSnack\Projects\test> node ace serve --watch
[ info ]  building project...
[ info ]  starting http server...
[1620320812541] INFO (test/10188 on roger): started server on 0.0.0.0:3333
[ encore ] Running webpack-dev-server ...
[ info ]  watching file system for changes
╭─────────────────────────────────────────────────╮
│                                                 │
│    Server address: http://127.0.0.1:3333        │
│    Watching filesystem for changes: YES         │
│    Running encore dev server: YES               │
│                                                 │
╰─────────────────────────────────────────────────╯
CREATE: public\assets\manifest.json
CREATE: public\assets\entrypoints.json
[ encore ]  DONE  Compiled successfully in 2060ms8:06:55 PM
[ encore ] webpack compiled successfully
PS C:\Users\McSnack\Projects\test> node ace serve --watch
[ info ]  building project...
[ info ]  starting http server...
[1620320854587] INFO (test/12136 on roger): started server on 0.0.0.0:3333
[ encore ] Running webpack-dev-server ...
[ info ]  watching file system for changes
╭─────────────────────────────────────────────────╮
│    Server address: http://127.0.0.1:3333        │
│    Running encore dev server: YES               │
│                                                 │
╰─────────────────────────────────────────────────╯
[ encore ] C:\Users\McSnack\Projects\test\node_modules\webpack-dev-server\lib\Server.js:559
      throw error;
      ^

Error: listen EADDRINUSE: address already in use :::8080
    at Server.setupListenHandle [as _listen2] (net.js:1318:16)
    at Server.listen (net.js:1452:7)
    at C:\Users\McSnack\Projects\test\node_modules\webpack-dev-server\lib\Server.js:771:30
    at processTicksAndRejections (internal/process/task_queues.js:93:5) {
  code: 'EADDRINUSE',
  errno: -4091,
  syscall: 'listen',
  address: '::',
  port: 8080
}

Expected outcome

CTRL + C should kill both, server and Webpack dev server

Actual outcome

Webpack dev server is not dying and has to be killed by PID otherwise port 8080 will be forever taken by proccess running in background

BONUS (a sample repo to reproduce the issue)

It happens with fresh project

Seems like it's issue in @adonis/assembler

If saving child proccess to AssetBundler instance in here:
https://github.com/adonisjs/assembler/blob/develop/src/AssetsBundler/index.ts#L106

this.webpackDevServerInstance = childProcess

And when storing encore to this.encore in
https://github.com/adonisjs/assembler/blob/develop/src/DevServer/index.ts#L201

Then should be able to kill it somewhere in here:
https://github.com/adonisjs/assembler/blob/develop/src/DevServer/index.ts#L188

But seems like none of the events run when killing with CTRL + C

Might be related to #2273 (comment)

It's most likely in wrong repo, but couldn't figure out to which repo to put it. Feel free to move to right place

@thetutlage
Copy link
Member

Will this option from execa help? https://github.com/sindresorhus/execa#cleanup

Maybe you have will have to try it yourself and help with a patch (Don't have windows with me) :)

@thetutlage thetutlage self-assigned this May 6, 2021
@thetutlage thetutlage added the Type: Question Needs clarification label May 6, 2021
@McSneaky
Copy link
Contributor Author

McSneaky commented May 6, 2021

Tried those options:

this.execaOptions = {
    preferLocal: true,
    buffer: false,
    stdio: 'pipe',
    localDir: this.projectRoot,
    cwd: this.projectRoot,
    cleanup: true, // Added
    detatched: false, // Added
    env: {
        FORCE_COLOR: 'true',
      ...this.env,
  },
};

But it had no affect. Still same :|

@McSneaky
Copy link
Contributor Author

McSneaky commented May 6, 2021

Found such interesting thing: https://github.com/sindresorhus/execa#windowshide

Modified the settings object:

this.execaOptions = {
  preferLocal: true,
  buffer: false,
  stdio: 'pipe',
  localDir: this.projectRoot,
  cwd: this.projectRoot,
  windowsHide: false, // Added
  env: {
      FORCE_COLOR: 'true',
      ...this.env,
  },
};

And it works all good now in Windows. Going to test out Linux too

There's also some interesting discussion about why they haven't switched default valu to false:
sindresorhus/execa#433 (comment)

In short: Can't set that option to false when using detached: true. Chaning default value would break it for people who rely on detached: true. Detached allows child proccess to keep running while main proccess exits, so it's not an option we want anyway. Maing it safe for us to set windowsHide: false

@McSneaky
Copy link
Contributor Author

McSneaky commented May 6, 2021

Tested on:
Windows ✔️
Native Linux (OpenSUSE) ✔️
WSL2 running Ubuntu ✔️

Setting windowsHide: false works as expected everywhere and doesn't have any side effects
It's not creating any additional (terminal) windows either, everything works like before, just CTRL+C kills webpack-dev-server with Adonis HTTP server

McSneaky added a commit to McSneaky/assembler that referenced this issue May 6, 2021
…C in Windows

Killing server with CTRL-C in dev mode in Windows leaves webpack-dev-server running on port 8080 and
never releases it

fix adonisjs/core#2562
@McSneaky
Copy link
Contributor Author

McSneaky commented May 6, 2021

Would be cool if someone can test it with Mac

thetutlage pushed a commit to adonisjs/assembler that referenced this issue May 8, 2021
…C in Windows (#26)

Killing server with CTRL-C in dev mode in Windows leaves webpack-dev-server running on port 8080 and
never releases it

fix adonisjs/core#2562
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Question Needs clarification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants