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

the version 1.0.22 do not search from the basePort #84

Closed
fubai opened this issue Aug 17, 2019 · 12 comments
Closed

the version 1.0.22 do not search from the basePort #84

fubai opened this issue Aug 17, 2019 · 12 comments

Comments

@fubai
Copy link

fubai commented Aug 17, 2019

the version 1.0.22 do not search from the basePort

@BarthesSimpson
Copy link
Contributor

It does, but instead of starting at basePort p and checking p, p+1, p+2... stopPort in order, it now checks a random sequence of ports in the range basePort...stopPort. This change was to fix an issue where calling getPort more than once in quick succession could lead to the same port being returned multiple times if the client had not finished binding by the time the second call ran its check (#79).

@bcfurtado
Copy link

bcfurtado commented Aug 17, 2019

Hi, this seems to be is a breaking change then. Previously, the expected behavior was that the basePort would be used as the initial default value.
Packages such as vue-cli and [webpack-dev] were relying on the old behavior.

$ npm list | grep portfinder
│ ├─┬ [email protected]
│ │ ├── [email protected] deduped

$ node --experimental-repl-await
> const portfinder = require('portfinder')
undefined
> portfinder.basePort
8000
> let port = await portfinder.getPortPromise()
undefined
> port
10211
$ npm list | grep portfinder
│ ├── [email protected] deduped
│ │ ├── [email protected] deduped
├─┬ [email protected]

$ node --experimental-repl-await
> const portfinder = require('portfinder')
undefined
> portfinder.basePort
8000
> let port = await portfinder.getPortPromise()
undefined
> port
8000

@jaredjj3
Copy link

It does, but instead of starting at basePort p and checking p, p+1, p+2... stopPort in order, it now checks a random sequence of ports in the range basePort...stopPort. This change was to fix an issue where calling getPort more than once in quick succession could lead to the same port being returned multiple times if the client had not finished binding by the time the second call ran its check (#79).

The change made does not solve the problem, since multiple calls of getPort can still return the same port. This "port collision" event is more probable as the range basePort...stopPort decreases to 1.

I recommend reverting this change or making it optional. The caller will then be responsible for port reservation.

@AndreasCag
Copy link

I recently configured webpack and was really frustrated with that behaviour.

As for me, I would like to have my app on a certain port most of the time, and if it is busy, then choose next port in a sequential order. It is easy to remember and convenient behaviour.

@ThibaultVlacich
Copy link

ThibaultVlacich commented Aug 18, 2019

It does, but instead of starting at basePort p and checking p, p+1, p+2... stopPort in order, it now checks a random sequence of ports in the range basePort...stopPort. This change was to fix an issue where calling getPort more than once in quick succession could lead to the same port being returned multiple times if the client had not finished binding by the time the second call ran its check (#79).

This absolutely makes no sense.

1/ There's no bug in the first place. If an application "call[s] getPort more than once in quick succession" there's a problem with the logic, as you should wait for a call to have returned a port, assign it, and then make an other call.
2/ Even if, it doesn't "fix" the issue, it just makes the probably of it happening very thin.

And seriously, releasing a breaking change like that as a "patch"...

@BarthesSimpson
Copy link
Contributor

1/ There's no bug in the first place. If an application "call[s] getPort more than once in quick succession" there's a problem with the logic, as you should wait for a call to have returned a port, assign it, and then make an other call.

It's not about waiting for the port to return - it's that whatever is binding to the returned port (e.g. a server) can take some time to bind even after the port is returned.

2/ Even if, it doesn't "fix" the issue, it just makes the probably of it happening very thin.

That's correct. It means any application that is binding and releasing a large number of ports dynamically can rely on a simple error handler in the downstream logic (i.e. if the attempt to bind encounters an EADDRINUSE, retry) which will be invoked roughly once every 32,000 calls rather than having to block on each binding attempt until successful binding is verified.

It seems like a lot of consumers are relying on the sequential scan though, and there is a PR out that reverts to making that the default behavior: #86

@ctstewart
Copy link

I use Vue Cli, and this completely broke devServer.port in vue.config.js. I ended up reverting to 1.0.21 of portfinder for now.

@lltx
Copy link

lltx commented Aug 19, 2019

Very bad

hjvedvik added a commit to gridsome/gridsome that referenced this issue Aug 19, 2019
Use -p 8080 as workaround for random port given by portfinder.

See: http-party/node-portfinder#84
@Qaick
Copy link

Qaick commented Aug 19, 2019

Also vuejs/vue-cli#4452

@pimterry
Copy link
Contributor

This has created bugs in my application too - I'm specifically depending on finding the first available port in the range, not a random port.

pimterry added a commit to httptoolkit/mockttp that referenced this issue Aug 19, 2019
v1.0.22 of portfinder changes to random selection, not sequential,
which makes the selected port change unnecessarily & unpredictably.

See http-party/node-portfinder#84
aarongranick-okta added a commit to okta/okta-oidc-js that referenced this issue Aug 19, 2019
@eriktrom
Copy link
Member

I will roll back immediately

@eriktrom
Copy link
Member

Reverted v1.0.22 - v1.0.23 is the same as v1.0.21 now

aarongranick-okta added a commit to okta/okta-oidc-js that referenced this issue Aug 19, 2019
… options (#529)

* buildConfigObject: support all config options

* Use lodash merge

* - portfinder 1.0.22 has a bug

http-party/node-portfinder#84
aarongranick-okta added a commit to okta/okta-vue that referenced this issue May 29, 2020
… options (#529)

* buildConfigObject: support all config options

* Use lodash merge

* - portfinder 1.0.22 has a bug

http-party/node-portfinder#84
alanraison pushed a commit to alanraison/cognito-oidc-pkce that referenced this issue Jun 3, 2020
… options (#529)

* buildConfigObject: support all config options

* Use lodash merge

* - portfinder 1.0.22 has a bug

http-party/node-portfinder#84
shuowu-okta pushed a commit to okta/okta-react-native that referenced this issue Jul 10, 2020
… options (#529)

* buildConfigObject: support all config options

* Use lodash merge

* - portfinder 1.0.22 has a bug

http-party/node-portfinder#84
brettritter-okta pushed a commit to okta/okta-react that referenced this issue Sep 24, 2020
… options (#529)

* buildConfigObject: support all config options

* Use lodash merge

* - portfinder 1.0.22 has a bug

http-party/node-portfinder#84
oleksandrpravosudko-okta pushed a commit to okta/okta-jwt-verifier-js that referenced this issue Aug 26, 2021
… options (#529)

* buildConfigObject: support all config options

* Use lodash merge

* - portfinder 1.0.22 has a bug

http-party/node-portfinder#84
denysoblohin-okta pushed a commit to okta/okta-oidc-middleware that referenced this issue Aug 27, 2021
… options (#529)

* buildConfigObject: support all config options

* Use lodash merge

* - portfinder 1.0.22 has a bug

http-party/node-portfinder#84
denysoblohin-okta pushed a commit to okta/okta-oidc-middleware that referenced this issue Aug 27, 2021
… options (#529)

* buildConfigObject: support all config options

* Use lodash merge

* - portfinder 1.0.22 has a bug

http-party/node-portfinder#84
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