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

fix(bosh): mark first request dead when second is done #418

Merged
merged 1 commit into from
Apr 28, 2022

Conversation

sunduev
Copy link
Contributor

@sunduev sunduev commented Apr 28, 2022

Logic to restart first request if second finishes earlier doesn't work.

strophejs/src/bosh.js

Lines 634 to 642 in 1877ecb

// if request 1 finished, or request 0 finished and request
// 1 is over Strophe.SECONDARY_TIMEOUT seconds old, we need to
// restart the other - both will be in the first spot, as the
// completed request has been removed from the queue already
if (reqIs1 ||
(reqIs0 && this._requests.length > 0 &&
this._requests[0].age() > Math.floor(Strophe.SECONDARY_TIMEOUT * this.wait))) {
this._restartRequest(0);
}

Request is never marked as dead in this case because we remove it on previous step.

strophejs/src/bosh.js

Lines 622 to 628 in 1877ecb

const valid_request = reqStatus > 0 && reqStatus < 500;
const too_many_retries = req.sends > this._conn.maxRetries;
if (valid_request || too_many_retries) {
// remove from internal queue
this._removeRequest(req);
Strophe.debug("request id "+req.id+" should now be removed");
}

This leads to long timeout instead of possible short secondary timeout in _processRequest

So we should save booleans before request is removed.
It was intended to work this way but did break on commit a7a1413

@jcbrand jcbrand merged commit fb79830 into strophe:master Apr 28, 2022
@jcbrand
Copy link
Contributor

jcbrand commented Apr 28, 2022

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants