From 00f0b06fcbe521878d9028158392424a7e45e900 Mon Sep 17 00:00:00 2001
From: Joey Parrish <joeyparrish@users.noreply.github.com>
Date: Thu, 26 Sep 2024 11:26:36 -0700
Subject: [PATCH] fix: Exclude TIMEOUT errors when disabling streams (#7369)

In #7368, we get stuck in a loop loading forever. This regression was
introduced in v4.4.0 and affects all v4.4, v4.5, v4.6, v4.7, and v4.8
releases, as well as v4.9.0-28, v4.9.2-caf1, v4.10.0-20, and v4.11.0-6.

The loop is composed of these elements:

1. an error that triggers disabling a stream
2. an error that doesn't resolve itself over time
3. an error that is slow enough to trigger that the first streams get
re-enabled
4. VOD content that doesn't change while we sit in the loop
5. enough streams to avoid exhausting them during the cycle

Only `TIMEOUT` errors can trigger this bug AFAICT, so we should exclude
those from the logic to disable streams. Note also that live streaming
already retries indefinitely by default, and that normal ABR logic will
change streams for us if we timeout due to a lack of bandwidth.

Disabling streams on `TIMEOUT` was suggested initially in #4764, but was
not a requirement of the OP. It was added out of caution in #4769, but
not really vetted. Because it was not ever explicitly needed, excluding
it is not a regression.

Closes #7368
---
 lib/media/streaming_engine.js | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/lib/media/streaming_engine.js b/lib/media/streaming_engine.js
index 6e52f27e61..2ef609251f 100644
--- a/lib/media/streaming_engine.js
+++ b/lib/media/streaming_engine.js
@@ -1937,8 +1937,12 @@ shaka.media.StreamingEngine = class {
     });
 
     if (!waitingForAnotherStreamToRecover) {
+      const maxDisabledTime = this.getDisabledTime_(error);
+      if (maxDisabledTime) {
+        shaka.log.debug(logPrefix, 'Disabling stream due to quota', error);
+      }
       const handled = this.playerInterface_.disableStream(
-          mediaState.stream, this.config_.maxDisabledTime);
+          mediaState.stream, maxDisabledTime);
       if (handled) {
         return;
       }
@@ -2786,18 +2790,26 @@ shaka.media.StreamingEngine = class {
    * @private
    */
   async handleStreamingError_(mediaState, error) {
+    const logPrefix = shaka.media.StreamingEngine.logPrefix_(mediaState);
+
     // If we invoke the callback right away, the application could trigger a
     // rapid retry cycle that could be very unkind to the server.  Instead,
     // use the backoff system to delay and backoff the error handling.
     await this.failureCallbackBackoff_.attempt();
     this.destroyer_.ensureNotDestroyed();
-    // Try to recover from network errors
-    if (error.category === shaka.util.Error.Category.NETWORK) {
+
+    // Try to recover from network errors, but not timeouts.
+    // See https://github.com/shaka-project/shaka-player/issues/7368
+    if (error.category === shaka.util.Error.Category.NETWORK &&
+        error.code != shaka.util.Error.Code.TIMEOUT) {
       if (mediaState.restoreStreamAfterTrickPlay) {
         this.setTrickPlay(/* on= */ false);
         return;
       }
       const maxDisabledTime = this.getDisabledTime_(error);
+      if (maxDisabledTime) {
+        shaka.log.debug(logPrefix, 'Disabling stream due to error', error);
+      }
       error.handled = this.playerInterface_.disableStream(
           mediaState.stream, maxDisabledTime);
 
@@ -2822,6 +2834,7 @@ shaka.media.StreamingEngine = class {
 
   /**
    * @param {!shaka.util.Error} error
+   * @return {number}
    * @private
    */
   getDisabledTime_(error) {