[HOTFIX] Change factor to 1 as any number higher cause an overflow

[sergicastellsague]

There's a bug in the library, in which after around 54 retries, the WebSocket does stop trying reconnecting.

It does stop because of the backoff ms.

var ms = _ms * Math.pow(_factor, this.attempts++);

The real fix would be changing the duration method. This is a hotfix.

[jumperchen]

According to the JS's implementation, it did this without any issue - https://github.com/mokesmokes/backo/blob/master/index.js#L24
https://github.com/mokesmokes/backo/blob/master/index.js#L36-L43

Do you have any test case?

[sergicastellsague]

Hey there,

We don't need tests for this. Pretty simple maths. If factor is 2 and the attemps counter is 50, the result of this math operation is 50^50. The number will fit in the "num", but if you check the code further, you'll see it's being used for the Duration in a timer. And Duration is initialized with an int value, not a num.

And an int has a pretty low limit that you reach easily.

[jumperchen]

But the returned value of the duration for the ms value should not be greater than the _max value from this line, shouldn't it? -

socket.io-client-dart/lib/src/manager.dart

Line 577 in 6980fbb

return Math.min(ms, _max);

[sergicastellsague]

Right. I just tried again and it does not fail that further.

I added some debugging lines:

I/flutter ( 4109): Attempts: 53. ms: 2536501090134643712 max: 5000 I/flutter ( 4109): returning: 5000
And next...
I/flutter ( 4109): Attempts: 54. ms: -5236583102765551616 max: 5000 I/flutter ( 4109): returning: -5236583102765551616

Then it all explodes in here:

I guess duration cannot handle a negative number

[jumperchen]

so, maybe we simply add the following code to avoid that case.

return Math.max(Math.min(ms, _max), 0);

[sergicastellsague]

That'd work as well. As a hotfix it seems to be a good approach as well.

What I am afraid of is that this backoff needs a rethought. What do you think about getting rid of the factor? This exponential value doesn't make much sense to me.

[jumperchen]

No, the fixed you did will be 1 forever (like Math.pow(1, 1000) => 1), but the original JS implementation will delay the duration each time it happens. I think it won't let the server to handle the reconnection frequently if the server is busying or hanging. So I prefer not to change the factor as the same as the JS implementation.