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
fix: sockLen was being miscalculated when removing sockets #60
Conversation
Order of operations issue with + and ternary statements: console.log(0 + '' ? 'a' : 'b') // a The code: freeLen + this.sockets[name] ? this.sockets[name].length : 0; Is equivalent to: (freeLen + this.sockets[name]) ? this.sockets[name].length : 0; When `this.sockets[name]` exists, it is an array, `(freeLen + this.sockets[name])` evaluates to a string, which evaluates to a string (true). When it does not, `(freeLen + this.sockets[name])` evaluates to `NaN` (false). Either way, `freeLen` is ignored.
Codecov Report
@@ Coverage Diff @@
## master #60 +/- ##
=======================================
Coverage 91.51% 91.51%
=======================================
Files 4 4
Lines 283 283
=======================================
Hits 259 259
Misses 24 24
Continue to review full report at Codecov.
|
This seems like a pretty important fix for potential bugs? Why no movement here? |
can you add a test case for this fix? |
Sorry for sleeping on this, I must've missed the notifications back in July. I took another look at this today to see how I could go about adding a test. I'll admit I'm not very familiar with the code; I found this issue doing static analysis with an AST parser on a project dependent on this one. I made the following change to see if I could better understand the effects this issue has: // [patch start]
var freeLen = this.freeSockets[name] ? this.freeSockets[name].length : 0;
var correctSockLen = freeLen + (this.sockets[name] ? this.sockets[name].length : 0);
var sockLen = freeLen + this.sockets[name] ? this.sockets[name].length : 0;
// [patch end]
if (correctSockLen !== sockLen) {
console.log(correctSockLen, sockLen, this.maxSockets);
}
if (this.requests[name] && this.requests[name].length && sockLen < this.maxSockets) {
// ...
} During testing, there's never a situation where Any suggestions would be appreciated. |
@cixel I see where is the bug now:
|
3.5.2 released @cixel Thanks. |
Thank you! Sorry for the delayed follow-up. |
Order of operations issue with + and ternary statements:
The code:
Is equivalent to:
When
this.sockets[name]
exists, it is an array,(freeLen + this.sockets[name])
evaluates to a string, which evaluates to a string (true). When it does not,(freeLen + this.sockets[name])
evaluates toNaN
(false).Either way,
freeLen
is ignored, which does not seem intended.