Avoid rapidly retrying failed requests (#5146)

After #5109 we retry
failed requests in a tight loop, instead of once every sync. When
requests are consistently failing, e.g. when /keys/uploads is failing
because of a duplicate OTK, this causes us to make many requests,
causing load on the server.

The fix is to reprocess the outgoing requests loop only if at least one
request succeeded in the last batch.

Fixes element-hq/element-web#31790

Author: andybalaam

src/rust-crypto/OutgoingRequestsManager.ts

@@ -127,11 +127,13 @@ export class OutgoingRequestsManager {
 
         const outgoingRequests: OutgoingRequest[] = await this.olmMachine.outgoingRequests();
 
+        let successes = 0;
         for (const request of outgoingRequests) {
             if (this.stopped) return;
             try {
                 await logDuration(this.logger, `Make outgoing request ${request.type}`, async () => {
                     await this.outgoingRequestProcessor.makeOutgoingRequest(request);
+                    successes++;
                 });
             } catch (e) {
                 // as part of the loop we silently ignore errors, but log them.
@@ -140,21 +142,22 @@ export class OutgoingRequestsManager {
             }
         }
 
-        // If we handled any requests this time, more may have been queued as
-        // part of that handling, so do an extra check to make sure we handle
-        // them immediately. See
-        // If we handled any requests this time, more may have been queued as
+        // If we successfully handled any requests this time, more may have been queued as
         // part of that handling.
         //
         // For example, we may have processed a `/keys/claim` request, which
         // meant the rust side could establish an Olm session and is now ready to
         // send out an `m.secret.send` message.
         // (See https://github.com/element-hq/element-web/issues/30988.)
         //
-        // So, if we have processed any requests, flag that we need make another
+        // So, if we have successfully processed any requests, flag that we need to make another
         // pass around the outgoing-requests loop, to make sure we handle any
         // pending requests immediately.
-        if (outgoingRequests.length > 0) {
+        //
+        // If all requests failed (or there weren't any) we don't want to retry them in a tight
+        // loop. They will be retried after the next sync.
+        // (See https://github.com/element-hq/element-web/issues/31790.)
+        if (successes > 0) {
             // We call doProcessOutgoingRequests but since we expect that we are
             // already processing outgoing requests, this call will not kick off
             // the processing loop, but just set `nextLoopDeferred` and return,