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

[email protected] breaks ember serve #8794

Closed
jelhan opened this issue Aug 17, 2019 · 10 comments
Closed

[email protected] breaks ember serve #8794

jelhan opened this issue Aug 17, 2019 · 10 comments

Comments

@jelhan
Copy link
Contributor

jelhan 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 [email protected] (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 [email protected] 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 Breaking change in 1.0.22 if port is > 40000 and stopPort is not set 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 [email protected] 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
Copy link
Member

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

@jelhan
Copy link
Contributor Author

jelhan 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
Copy link
Member

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

@larryh
Copy link

larryh 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
Copy link

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

@jelhan
Copy link
Contributor Author

jelhan commented Aug 18, 2019

  1. As soon as one upgrades to [email protected] (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
Copy link

imkathir commented Aug 19, 2019

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

fran-worley added a commit to adopted-ember-addons/ember-light-table that referenced this issue Aug 19, 2019
@rwjblue
Copy link
Member

rwjblue 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
Copy link
Contributor

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)

@locks
Copy link
Contributor

locks commented Mar 31, 2020

We are currently on 1.0.25, so I think it's safe to say the PR linked just before addressed this problem? Thanks for the discussion everyone!

@locks locks closed this as completed Mar 31, 2020
RobbieTheWagner added a commit to adopted-ember-addons/ember-light-table that referenced this issue Feb 9, 2021
* replace `sendAction` with modern callable methods

* make easy jquery fixes

* Release v2.0.0-beta.0

* Release v2.0.0-beta.1

* removed jquery

* yarn upgrade

* Update ember-scrollable

* Assert and Test compatibility with LTS 3.4

* Officially drop support for node 4 in package.json

* bump vertical-collection to v1.0.0-beta.13

* Replace merge with assign (#673)

* bump vertical-collection to v1.0.0-beta.13

* replace merge with assign

* ensure ember-scrollable updates when rows are updated (#677)

* refactor: Remove sendAction() calls (#686)

* release 2.0.0-beta.3

* Replace propertyWillChange/propertyDidChange with notifyPropertyChange and add polyfill for < Ember 3.1 (#692)

Fixes #660

* update changelog for 2.x

* Update ember-in-viewport, ember-wormhole, move ember-composable-helpers to devDependencies (#693)

* move ember composable helpers to dev dependencies and bump version  see #524

* bump ember wormhole

* add ember lts 3.8 to travis

* remove ember-native-dom-helpers testing add-on

* update multiple table test from 1-x branch to remove native-dom-helpers add-on

* [breaking] rename inViewport closure action to enterViewport

paving the way to update ember-in-viewport dependency

* fixes infinite loop on onScrolledToBottom

* remove unused rows property from lt-infinity

* bump ember-in-viewport version and refactor lt-infinity component not to use mixin

* make lt-body.inViewport event deprecated in favour of enterViewport and test

* Drop node 6 support as end of line Apr 2019 (#698)

LGTM! Node 8 it latest Maintenance LTS and will be EOL in December

* Bump Ember CLI to 3.8 and update other dependencies (#696)

* bump ember cli to lts 3.8 update other dev dependencies

also apply some ember add-on blueprint changes

* fix handlebar lint errors

Note - unsure why 'no-inline-styles' config isn’t being applied. For now I’ve manually disabled the rule in applicable templates

* disable js no-observer rule

* update eslint-plugin-ember-suave and fix lint errors

* upgrade ember-mirage (used for dummy app)

* upgrade remaining addons

* set minimum supported ember version at 2.18 (#700)

see community survey results - https://emberjs.com/ember-community-survey-2019/

* migrate to using lerna-changelog (#697)

* update changelog [ci skip]

* BREAKING drop support for ember 2.18 (#713)

* [BREAKING] drop support for ember 2.18

* fix travis - ember release can fail

* as we use the compute helper we need to restore ember-composable-helpers as a dependency (#714)

* [BREAKING] fixes #444 #662 converts ES6 native classes to ember objects (#701)

This is a breaking change as it converts the new Class() syntax to Class.create().
In addition, the only way I could get these to work was to convert the positional arguments to named args in an options hashes:

`new Table(columns, rows)` => `Table.create(columns: columns, rows: rows)`
`new Row(content, options)` => `Row.create({ content: })`
`new Column(opts)` => `Column.create(opts)`

We’ve had to replace the native constructor methods with emberObject init methods as per the official docs which should also fix #699. As a result we’ve removed the Ember 2.12 ‘hacks’ which shouldn’t be an issue as we’ve bumped ember version minimum version to 2.18.

* Bump to ember cli 3.12 and update dependencies (#716)

* bump addons

* update ember cli to 3.12

* drop ember-release from allowed travis failures

* use yarn not npm and remove unused ember-cli-changelog

* update ember changelog

* [ci skip] fix dummy app port following update to ember cli 3.12

see ember-cli/ember-cli#8794

* release v2.0.0-beta.4

* [ci-skip] update changelog for beta 4 release

* replace volatile computed properties with native getters (#718)

* Add a guard to check if scaffolding cells exist

* Always render scaffolding row in table header

* Test lt-scaffolding-row

* Update ember-scrollable version to jquery-less

* bump ember-code-snippet for dummy app

* update travis to include recent LTS versions of ember

* fix lint issues

* bump ember cli to 3.16.1 and fix/ warn easy js lint errors disable other rules

* bump dependency addons

* [ci skip] bump changelog and yuidoc addons

* [ci-skip] update changelog

* fix deprecation warning for overridden rowspan cp

* remove unsafe style attribute warnings

* make compute own helper then drop ember-composable-helpers as dependency

* Remove debugging line in compute helper

* Drop support to Ember 3.4 and 3.5

Related #739

* add testing for ember 3.20

* bump addons

* drop ember-suave eslint to fix travis

# Conflicts:
#	package.json
#	yarn.lock

* [ci-skip] update ember version in readme

* v3.16.1...v3.20.0

* drop support for ember <3.12

* drop this.get

Co-authored-by: Donald Wasserman <[email protected]>
Co-authored-by: Alex Alvarez <[email protected]>
Co-authored-by: Gaurav Munjal <[email protected]>
Co-authored-by: Fran Worley <[email protected]>
Co-authored-by: Michal Bryxí <[email protected]>
Co-authored-by: mmadsen2 <[email protected]>
Co-authored-by: Tomasz Węgrzyn <[email protected]>
Co-authored-by: Richard Viney <[email protected]>
Co-authored-by: gorzas <[email protected]>
RobbieTheWagner added a commit to adopted-ember-addons/ember-light-table that referenced this issue Jul 8, 2022
* replace `sendAction` with modern callable methods

* make easy jquery fixes

* Release v2.0.0-beta.0

* Release v2.0.0-beta.1

* removed jquery

* yarn upgrade

* Update ember-scrollable

* Assert and Test compatibility with LTS 3.4

* Officially drop support for node 4 in package.json

* bump vertical-collection to v1.0.0-beta.13

* Replace merge with assign (#673)

* bump vertical-collection to v1.0.0-beta.13

* replace merge with assign

* ensure ember-scrollable updates when rows are updated (#677)

* refactor: Remove sendAction() calls (#686)

* release 2.0.0-beta.3

* Replace propertyWillChange/propertyDidChange with notifyPropertyChange and add polyfill for < Ember 3.1 (#692)

Fixes #660

* update changelog for 2.x

* Update ember-in-viewport, ember-wormhole, move ember-composable-helpers to devDependencies (#693)

* move ember composable helpers to dev dependencies and bump version  see #524

* bump ember wormhole

* add ember lts 3.8 to travis

* remove ember-native-dom-helpers testing add-on

* update multiple table test from 1-x branch to remove native-dom-helpers add-on

* [breaking] rename inViewport closure action to enterViewport

paving the way to update ember-in-viewport dependency

* fixes infinite loop on onScrolledToBottom

* remove unused rows property from lt-infinity

* bump ember-in-viewport version and refactor lt-infinity component not to use mixin

* make lt-body.inViewport event deprecated in favour of enterViewport and test

* Drop node 6 support as end of line Apr 2019 (#698)

LGTM! Node 8 it latest Maintenance LTS and will be EOL in December

* Bump Ember CLI to 3.8 and update other dependencies (#696)

* bump ember cli to lts 3.8 update other dev dependencies

also apply some ember add-on blueprint changes

* fix handlebar lint errors

Note - unsure why 'no-inline-styles' config isn’t being applied. For now I’ve manually disabled the rule in applicable templates

* disable js no-observer rule

* update eslint-plugin-ember-suave and fix lint errors

* upgrade ember-mirage (used for dummy app)

* upgrade remaining addons

* set minimum supported ember version at 2.18 (#700)

see community survey results - https://emberjs.com/ember-community-survey-2019/

* migrate to using lerna-changelog (#697)

* update changelog [ci skip]

* BREAKING drop support for ember 2.18 (#713)

* [BREAKING] drop support for ember 2.18

* fix travis - ember release can fail

* as we use the compute helper we need to restore ember-composable-helpers as a dependency (#714)

* [BREAKING] fixes #444 #662 converts ES6 native classes to ember objects (#701)

This is a breaking change as it converts the new Class() syntax to Class.create().
In addition, the only way I could get these to work was to convert the positional arguments to named args in an options hashes:

`new Table(columns, rows)` => `Table.create(columns: columns, rows: rows)`
`new Row(content, options)` => `Row.create({ content: })`
`new Column(opts)` => `Column.create(opts)`

We’ve had to replace the native constructor methods with emberObject init methods as per the official docs which should also fix #699. As a result we’ve removed the Ember 2.12 ‘hacks’ which shouldn’t be an issue as we’ve bumped ember version minimum version to 2.18.

* Bump to ember cli 3.12 and update dependencies (#716)

* bump addons

* update ember cli to 3.12

* drop ember-release from allowed travis failures

* use yarn not npm and remove unused ember-cli-changelog

* update ember changelog

* [ci skip] fix dummy app port following update to ember cli 3.12

see ember-cli/ember-cli#8794

* release v2.0.0-beta.4

* [ci-skip] update changelog for beta 4 release

* replace volatile computed properties with native getters (#718)

* Add a guard to check if scaffolding cells exist

* Always render scaffolding row in table header

* Test lt-scaffolding-row

* Update ember-scrollable version to jquery-less

* bump ember-code-snippet for dummy app

* update travis to include recent LTS versions of ember

* fix lint issues

* bump ember cli to 3.16.1 and fix/ warn easy js lint errors disable other rules

* bump dependency addons

* [ci skip] bump changelog and yuidoc addons

* [ci-skip] update changelog

* fix deprecation warning for overridden rowspan cp

* remove unsafe style attribute warnings

* make compute own helper then drop ember-composable-helpers as dependency

* Remove debugging line in compute helper

* Drop support to Ember 3.4 and 3.5

Related #739

* add testing for ember 3.20

* correct travis

* bump addons

* v3.16.1...v3.20.0

* bump ember version to 3.20

* fix travis.yml

* fix eslintrc

* replace one-way-controls with ember-power-select and ember input

* convert to angle bracket syntax

* fix ember data deprecations

* fix ember-power-select commit

* replace actions for fn/on non shared table

* replace table-shared mixin with base-table component

* convert action to fn for base-table actions

* replace remainder of action with on/fn

* bump dummy app addons

* Drop support to Ember 3.4 and 3.5

Related #739

* add testing for ember 3.20

* bump addons

* drop ember-suave eslint to fix travis

# Conflicts:
#	package.json
#	yarn.lock

* [ci-skip] update ember version in readme

* v3.16.1...v3.20.0

* drop support for ember <3.12

* drop this.get

* Released 3.0.0-beta.0

* ci: Add github actions with embroider test scenarios

* build: Drop Ember 3.12 support

BREAKING CHANGE: Dropped support for Ember 3.12

* Node 12

* fix: Fix linting errors

* ci: Allow embroider tests to fail

Tests pass locally but not in when running with GitHub actions

* docs: Remove Embroider safe and optimized from readme

CI fails in GitHub actions

* fix: Change to @faker/fakerjs to fix missing avatars

* Revert "fix: Change to @faker/fakerjs to fix missing avatars"

@faker/fakerjs requires Node 14. This reverts commit 81ba515.

Co-authored-by: Donald Wasserman <[email protected]>
Co-authored-by: Alex Alvarez <[email protected]>
Co-authored-by: Gaurav Munjal <[email protected]>
Co-authored-by: Fran Worley <[email protected]>
Co-authored-by: Michal Bryxí <[email protected]>
Co-authored-by: mmadsen2 <[email protected]>
Co-authored-by: Tomasz Węgrzyn <[email protected]>
Co-authored-by: Richard Viney <[email protected]>
Co-authored-by: gorzas <[email protected]>
Co-authored-by: maxwondercorn <[email protected]>
Co-authored-by: Robert Wagner <[email protected]>
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

No branches or pull requests

8 participants