Randomize selection order of tested ports

[BarthesSimpson]

Addresses #79 by creating an array of candidate ports to test and randomly shuffling it. The remaining logic is unchanged, except that the check for whether a tested port exceeds the global highestPort is moved from testPort to getPort (i.e. we exclude any port higher than highestPort from the initial list of candidate ports).

[eriktrom]

Awesome pr.

Looks like we can delete exports.nextPort - it was only used here https://github.com/http-party/node-portfinder/search?q=nextPort&unscoped_q=nextPort which u refactored away.

Lets remove that export (it should have been an internal function in the first place and i don't see any reason a lib would currently depend on it as it does nothing useful for upstream consumers).

Then i'll merge, thanks.

[eriktrom]

also, now that we're randomizing lets change exports.highestPort = 65535 to exports.highestPort = 40000 - explanation below, tl;dr - so we don't introduce a breaking change.

---

why:

macOS binds its touch bar to a random port in the range from 40,000 to ~60,000 and w/o a refactor to echo some text through a port said to be open, the socket will still bind even though it's not really usable.

There are client side cli's like ember-cli that changed their portfinder.basePort to 7020 to deal with this -- if 7020 isn't available, it would iterate up, but never hit 40,000(or at least not likely and no one has filed an issue since, and there were a surprising number of reports when it was too high and the new macbook pro's came out couple years back).

Thus, now that we're randomizing and apple is randomizing it's possible (someday, probably a year from now, lol) where this collision could take place again. Until I do (or don't) echo text through the port to actually guarantee its open, 40,000 - 65,000 should just not be tried unless the consumer specifically passes in an upper bound option.

Thanks :)

[BarthesSimpson]

That's a good callout. One other thing: there's now a small probability that getPorts will contain duplicates. e.g. if a user calls getPorts(5, ...) there's a ~1/2,000 chance that the returned array will include a non-unique element. For getPorts(20, ...) it's ~1/200. It could be made deterministic by keeping track of the ports that have been discovered on each iteration and re-running getPort if a duplicate is found, but that would make the implementation a bit less clear. Let me know if you think it's worth it.

[eriktrom]

good catch and nice inline docs :)

i think its fine. using getPorts was only useful in small numbers unless you had really good luck guessing the setTimeout you needed to add around it before you could actually use the ports in your app. the bigger the number, the less useful it was, meaning the probability of the randomized algo is not the determining factor in such cases. In smaller numbers, the same holds -- in the tests this became a problem so i know it's real in practice -- it's gotten better with node versions, but releasing ports is still very random w/ respect to time and the system in which getPorts is ran.

so it's totally justified to say it's not worth it here. the real fix is to call getPort then use the port, recurse, in your app, if you want high probability of correct behavior - which is really the essence of this pr in the first place (interesting that we looped back around to the same problem u fixed, just in different prose).

so i think we're good, thanks again for the pr 👍