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

Showing port number are different between Vite and Kit if 3000 is already in use #2467

Closed
baseballyama opened this issue Sep 21, 2021 · 11 comments · Fixed by #2507
Closed

Showing port number are different between Vite and Kit if 3000 is already in use #2467

baseballyama opened this issue Sep 21, 2021 · 11 comments · Fixed by #2507
Labels
bug Something isn't working
Milestone

Comments

@baseballyama
Copy link
Member

baseballyama commented Sep 21, 2021

Describe the bug

When I run npm run dev in kit project,
Different port shows if 3000 is already in use as you can see from the log.

Reproduction

After use 3000 port by some process, run this commands.

yes "" | npm init svelte my-app
cd my-app
npm install
npm run dev

Logs

vite v2.5.7 dev server running at:

  > Local: http://localhost:3001/
  > Network: use `--host` to expose

  SvelteKit v1.0.0-next.169

  network: not exposed
  network: not exposed
  local:   http://localhost:3000

  Use --host to expose server to other devices on this network

System Info

System:
    OS: Windows 10 10.0.19043
    CPU: (8) x64 Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz  
    Memory: 2.55 GB / 15.86 GB
  Binaries:
    Node: 14.17.0 - C:\Program Files\nodejs\node.EXE       
    npm: 6.14.13 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1023.0), Chromium (93.0.961.52)
    Internet Explorer: 11.0.19041.1202
  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.170
    svelte: ^3.34.0 => 3.42.6

Severity

annoyance

Additional Information

Maybe packages\kit\src\cli.js#welcome should be removed most of the code??

https://github.com/sveltejs/kit/blob/master/packages/kit/src/cli.js#L217

And the output should be like this?

  vite v2.5.7 dev server running at:

  > Local: http://localhost:3000/
  > Network: use `--host` to expose

  SvelteKit v1.0.0-next.169

  Use --host to expose server to other devices on this network

Thank you.

@bluwy
Copy link
Member

bluwy commented Sep 21, 2021

The duplicated welcome messages likely came from #2232, I think the best case is if we can replace vite v2.5.7 dev server with SvelteKit v1.0.0-next.169, though that's tricky. Otherwise having a SvelteKit v1.0.0-next.169 message below vite's might be fine too.

@bluwy bluwy added the bug Something isn't working label Sep 21, 2021
@benmccann
Copy link
Member

I thought kit would exit if the port were already in use:

process.exit(1);

I'm not sure why it's still running. But it looks like the message is being printed here:

function welcome({ port, host, https, open }) {

@benmccann benmccann added this to the 1.0 milestone Sep 21, 2021
@baseballyama
Copy link
Member Author

@benmccann

When I create #2441, you said we can use a port dynamically after merge #2232.
So I thought that now running on 3001 is expected behavior.
But the actual spec is that it should stop a server if 3000 is already in use??

And if we should stop a server if port 3000 is already in use,
perhaps we need to update port-authority as already mentioned at #948 (comment).
(But still, I didn't dig so I need to check more according to your answer.)

@benmccann
Copy link
Member

I don't think SvelteKit should have port checking code since it already exists in Vite. But it's there and it's surprising to me that it's not working on your machine. It does work for me on Linux. I guess it doesn't work on Windows. But anyway, we should probably remove that code

@benmccann
Copy link
Member

I sent a PR to Vite to remove the log messages that it's printing: vitejs/vite#5016

We should still remove our port choosing logic in favor of Vite's strictPort option and cleanup some stuff on our end, but maybe I will wait to see what happens with the Vite PR first since that may affect exactly what changes on our end

@bluwy
Copy link
Member

bluwy commented Sep 22, 2021

Do we want to remove the log messages entirely? We wouldn't know what the URL is ahead of time if strictPort: false (which is default for Vite). The same may apply to the network URL too, unless we duplicate Vite's code.

@benmccann
Copy link
Member

I was thinking we'd update our code to check what port the server was running on after it started

@bluwy
Copy link
Member

bluwy commented Sep 22, 2021

Ah if there's a way to get the port after createServer. It would be fine then.

@baseballyama
Copy link
Member Author

This is just an update current situation.

vitejs/vite#5016 was merged (Thank you @benmccann).
So I tried to run dev and preview commands.
And I share the result.

dev

  SvelteKit v1.0.0-next.174

  network: not exposed
  network: not exposed
  local:   http://localhost:3000

  Use --host to expose server to other devices on this network

The duplicate message is gone👍.
But if 3000 port is used, local message is wrong.

So we should change args for welcome function.
And we should remove check_port.

const { server } = watcher.vite.config;
welcome({ port: server.port, host: server.host, https: host.https, open });

Preview

If 3000 is free, it works fine.

  SvelteKit v1.0.0-next.174

  network: not exposed
  network: not exposed
  local:   http://localhost:3000

  Use --host to expose server to other devices on this network

But 3000 is already in use, below error comes (I'm using Windows10).

  SvelteKit v1.0.0-next.174

  network: not exposed
  network: not exposed
  local:   http://localhost:3000

  Use --host to expose server to other devices on this network


events.js:353
      throw er; // Unhandled 'error' event
      ^

Error: listen EADDRINUSE: address already in use 127.0.0.1:3000
    at Server.setupListenHandle [as _listen2] (net.js:1318:16)
    at listenInCluster (net.js:1366:12)
    at GetAddrInfoReqWrap.doListen [as callback] (net.js:1503:7)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:69:8)
Emitted 'error' event on Server instance at:
    at emitErrorNT (net.js:1345:8)
    at processTicksAndRejections (internal/process/task_queues.js:82:21) {
  code: 'EADDRINUSE',
  errno: -4091,
  syscall: 'listen',
  address: '127.0.0.1',
  port: 3000
}
 ELIFECYCLE  Command failed with exit code 1.

Preview doesn't use Vite, right?
If yes, to have the same behavior between dev and preview needs to take effort I think.
So if we regard this preview behavior as correct, then we don't need to do anything.

@bluwy
Copy link
Member

bluwy commented Sep 26, 2021

Thanks to vitejs/vite#5014, we can use Vite's preview for that as well. Though that feature is still in beta, and it would take another week (I think) before Vite releases a stable 2.6.0 version.

For dev I think we should follow your proposed solution. For preview however, we could try a best-effort fix for now or leave it as is and wait for Vite (?).

@baseballyama
Copy link
Member Author

Oh sorry, I forgot about vitejs/vite#5014.

We should both dev and preview depend on Vite to avoiding program duplication.
So I think we should wait for Vite's preview function to be ready to use in my opinion.
And now we can keep current code of preview. (Do nothing for now for preview)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants