FEATURE: when we fail to ship topic timings attempt to retry (PR #10916)

This change amends it so

  1. Topic timings are treated as background requests and subject to more aggressive rate limits.

  2. If we notice an error when we ship timings we back off exponentially

The commit allows 405, 429, 500, 501, 502, 503 and 504 errors to be retried.

500+ errors usually happen when self hosters are rebuilding or some other weird condition.

405 happens when site is in readonly. 429 happens when user is rate limited.

The retry cadence is hardcoded in AJAX_FAILURE_DELAYS, longest delay is 40 seconds, we may consider enlarging it.

After the last delay passes we give up and do not write timings to the server.

GitHub

FWIW you can rewrite this like it:

const [timings, topicTime, topicid] = item;

I would recommend making a new class, or use an object literal instead of a tuple here. It’s confusing to see all the array indices above elem[2], found[1] and not know what attributes they represent.

Why do we remove the object only to add it again below? (unshift)

Removing an object in JS is way slower than modifying it in place. It would be less work to not remove it, and only unshift a record that was not found in the else clause.

If you initialized this._consolidatedTimings and this._blockSendingToServerTill in the constructor, not only would it be more performant due to the way javascript handles object shapes, but you wouldn’t need to check if they exist every time.

Or if you use an object as I suggested above: const { timings, topictime, topicId } = item;

This is suspicious. -1 means no match, so >= -1 would mean “no match or any match”.

I suspect you meant !== -1 or > -1

We could console.warn/log here. It might be useful if we are trying to help someone debug something and they can open it up and see the errors.

done, moved to an object

done moved to an object

Yeah I changed it so we use push/pop instead so freshest is at the end. The means we lean now on findIndex which is not going to work on ie11.

sure done!

great catch!

Sure, added. We have been eating these errors now for a couple of years but this code is more complicated so more debugging can help

Looks really nice now!

Thanks Robin!