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

portfinder@1.0.22 breaks ember serve #8794

Open
jelhan opened this issue Aug 17, 2019 · 9 comments

Comments

@jelhan
Copy link

commented Aug 17, 2019

portfinder just released a 1.0.22 version. It breaks ember serve in two ways:

  1. As soon as one upgrades to portfinder@1.0.22 (or generate a new ember project / addon) ember serve always fails with

    Port 4200 is already in use.

    This is also true for all other ports passed to ember serve, e.g. ember serve --port 4300. I'm not sure why this happens.

  2. If port is > 40000 portfinder@1.0.22 throws:

    Provided options.stopPort(40000is less than options.startPort (40001)

    This is caused by default port being reduced from 65535 to 40000. This is reported upstream as http-party/node-portfinder#85.

This will affect everybody who

  • does not lock subdependencies
  • upgrades ember-cli
  • generates a new ember project (app or addon)

Ember CLI uses a version range that allows portfinder@1.0.22 for a few year. So nearly all active versions will be affected.

If using yarn, it's resolutions feature could be used as a quick work-a-round. To do so add a resolutions key to your package.json:

"resolutions": {
  "ember-cli/portfinder": "1.0.21"
}

Do not miss to run yarn install afterwards.

If you are not sure which version of portfinder is used in your project, run yarn why portfinder.

@kellyselden

This comment has been minimized.

Copy link
Member

commented Aug 17, 2019

Thanks for digging into this. Hopefully they resolve it quickly.

@jelhan

This comment has been minimized.

Copy link
Author

commented Aug 17, 2019

Another work-a-round was reported in a question on StackOverflow: ember serve --port 0 In that case portfinder picks a random port, which seems to work fine.

@kellyselden

This comment has been minimized.

Copy link
Member

commented Aug 17, 2019

Another workaround is to generate an older lockfile via npm install --before 2019-08-16.

@larryh

This comment has been minimized.

Copy link

commented Aug 17, 2019

Hi,

I ran into the same problem when upgrading from ember 3.11 to 3.12.

I tried @kellyselden suggestion for us npm users: npm install portfinder --before 2019-08-16 and it rolled back my just-updated version of porfinder from 1.0.22 to 1.0.21 => Thank you, sir!

@BarthesSimpson

This comment has been minimized.

Copy link

commented Aug 18, 2019

This fix is out for review: http-party/node-portfinder#86

@jelhan

This comment has been minimized.

Copy link
Author

commented Aug 18, 2019

  1. As soon as one upgrades to portfinder@1.0.22 (or generate a new ember project / addon) ember serve always fails with

    Port 4200 is already in use.

    This is also true for all other ports passed to ember serve, e.g. ember serve --port 4300. I'm not sure why this happens.

This is caused by http-party/node-portfinder#84. Ember CLI relies on sequential order of attempts. Especially if not called with --port 0 it asserts that the default port 4200 or the port defined by --port is free. If not it throws the "Port ??? is already in use." error.

return getPort({ port: commandOptions.port, host: commandOptions.host }).then(foundPort => {
if (commandOptions.port !== foundPort && commandOptions.port !== 0) {
let message = `Port ${commandOptions.port} is already in use.`;
return Promise.reject(new SilentError(message));
}

Actually the code here is less than optimal. We use getPort method to verify if a specific port is free but don't specify a stopPort. Therefore it searches for an open port between commandOptions.port and it's default highest port (40000 for 1.0.22 and 65535 for older versions). But we are not interested in any other port than commandOptions.port (unless that one is 0).

Should be refactored to something like

let options = {
  host: commandOptions.host,
};

// if a specific port is given, we want to use that one or fail
if (commandOptions.port !== 0) {
  options.port: commandOptions.port,
  options.stopPort: commandOptions.port,
}

try {
  let foundPort = await getPort({ port: commandOptions.port, stopPort: commandOptions.port })
} catch (err) {
  let message = commandOptions.port !== 0 ?
    `Port ${commandOptions.port} is already in use.` : 
    `All ports above ${commandOptions.port} are already in use.`;
  return Promise.reject(new SilentError(message));
}

I'm not even sure if we need getPort() for the scenario commandOptions.port !== 0 at all. It may be fine just to try to bind to the given port and fail with a meaningful error if it's already in use. That would also prevent us from race conditions. In current implementation a port might be free when calling PortFinder.getPort() but might not be anymore when actually trying to bin express server to it.

@imkathir

This comment has been minimized.

Copy link

commented Aug 19, 2019

I've encountered this issue in my app as well.

fran-worley added a commit to offirgolan/ember-light-table that referenced this issue Aug 19, 2019
@rwjblue

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

This has been somewhat mitigated (@eriktrom published a 1.0.23 with the code from 1.0.21 while we work through the failures here).

@eriktrom

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2019

This has been somewhat mitigated (@eriktrom published a 1.0.23 with the code from 1.0.21 while we work through the failures here).

yes, more precisely, 1.0.23 is 1.0.21, exactly

(context: a good idea from a contributor had a couple issues, sorry all. Moving forward, we will be much more strict in releasing any changes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.