From ee2338a33bc4bc5bb4be6c7457bb8d245ece4a0e Mon Sep 17 00:00:00 2001
From: Jakob Ackermann <jakob.ackermann@overleaf.com>
Date: Thu, 17 Apr 2025 12:31:44 +0100
Subject: [PATCH] [web] align criteria for fallback to clsi-cache (#24970)

* [web] rename helper for browser cache bug, avoid confusion w/ clsi-cache

* [web] align criteria for fallback to clsi-cache

Notably, avoid the fallback from inside pdf-caching when disabled.

GitOrigin-RevId: 3fd918de14eef59c45c28cc5b5e256048cb91528
---
 .../pdf-preview/util/pdf-caching-transport.js |  1 +
 .../features/pdf-preview/util/pdf-caching.js  | 23 +++++++++++++------
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.js b/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.js
index 3bebc980ae..166a2b0d94 100644
--- a/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.js
+++ b/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.js
@@ -175,6 +175,7 @@ export function generatePdfCachingTransportFactory() {
         prefetchLargeEnabled,
         cachedUrlLookupEnabled,
         abortSignal,
+        canTryFromCache,
         fallbackToCacheURL: getOutputPDFURLFromCache(),
       })
         .catch(err => {
diff --git a/services/web/frontend/js/features/pdf-preview/util/pdf-caching.js b/services/web/frontend/js/features/pdf-preview/util/pdf-caching.js
index 77e7d35a2b..a13c74aa2a 100644
--- a/services/web/frontend/js/features/pdf-preview/util/pdf-caching.js
+++ b/services/web/frontend/js/features/pdf-preview/util/pdf-caching.js
@@ -30,7 +30,7 @@ const CACHE_NO_STORE = 'no-store'
  * @param {string} url
  * @param {RequestInit} init
  */
-async function fetchWithCacheFallback(url, init) {
+async function fetchWithBrowserCacheFallback(url, init) {
   try {
     return await fetch(url, init)
   } catch (err) {
@@ -515,7 +515,7 @@ function getDynamicChunkInit({ file, start, end, signal }) {
 export async function fallbackRequest({ file, url, start, end, abortSignal }) {
   try {
     const init = getDynamicChunkInit({ file, start, end, signal: abortSignal })
-    const response = await fetchWithCacheFallback(url, init)
+    const response = await fetchWithBrowserCacheFallback(url, init)
     checkChunkResponse(response, end - start, init)
     return await response.arrayBuffer()
   } catch (e) {
@@ -590,6 +590,7 @@ function skipPrefetched(chunks, prefetched, start, end) {
  * @param {Map<string, {url:string,init:RequestInit}>} cachedUrls
  * @param {Object} metrics
  * @param {boolean} cachedUrlLookupEnabled
+ * @param {() => boolean} canTryFromCache
  * @param {string} fallbackToCacheURL
  * @param {Object} file
  */
@@ -600,6 +601,7 @@ async function fetchChunk({
   cachedUrls,
   metrics,
   cachedUrlLookupEnabled,
+  canTryFromCache,
   fallbackToCacheURL,
   file,
 }) {
@@ -614,7 +616,10 @@ async function fetchChunk({
     // We memorize the previous browser cache keys in `cachedUrls`.
     try {
       oldUrl.init.signal = init.signal
-      const response = await fetchWithCacheFallback(oldUrl.url, oldUrl.init)
+      const response = await fetchWithBrowserCacheFallback(
+        oldUrl.url,
+        oldUrl.init
+      )
       if (response.status === 200) {
         checkChunkResponse(response, estimatedSize, init)
         metrics.oldUrlHitCount += 1
@@ -632,7 +637,7 @@ async function fetchChunk({
   }
   let response
   try {
-    response = await fetchWithCacheFallback(url, init)
+    response = await fetchWithBrowserCacheFallback(url, init)
     checkChunkResponse(response, estimatedSize, init)
     if (chunk.hash) {
       delete init.signal // omit the signal from the cache
@@ -642,9 +647,10 @@ async function fetchChunk({
     if (chunk.hash) {
       cachedUrls.delete(chunk.hash)
     }
-    const isMissing = response?.status === 404 || response?.status === 416
     const hasOthersCached = cachedUrls.size > 0
-    if (isMissing && hasOthersCached) {
+    const info = { url, init, statusCode: response?.status }
+    const errTagged = OError.tag(err1, 'add info for canTryFromCache', info)
+    if (hasOthersCached && canTryFromCache(errTagged) && fallbackToCacheURL) {
       // Only try downloading chunks that were cached previously
       file.ranges = file.ranges.filter(r => cachedUrls.has(r.hash))
       // Try harder at fetching the chunk, fallback to cache
@@ -659,7 +665,7 @@ async function fetchChunk({
         })
       }
       try {
-        response = await fetchWithCacheFallback(url, init)
+        response = await fetchWithBrowserCacheFallback(url, init)
         checkChunkResponse(response, estimatedSize, init)
       } catch (err2) {
         throw err1
@@ -816,6 +822,7 @@ class Timer {
  * @param {boolean} prefetchLargeEnabled
  * @param {boolean} tryOldCachedUrlEnabled
  * @param {AbortSignal} abortSignal
+ * @param {() => boolean} canTryFromCache
  * @param {string} fallbackToCacheURL
  */
 export async function fetchRange({
@@ -832,6 +839,7 @@ export async function fetchRange({
   prefetchLargeEnabled,
   cachedUrlLookupEnabled,
   abortSignal,
+  canTryFromCache,
   fallbackToCacheURL,
 }) {
   const timer = new Timer()
@@ -974,6 +982,7 @@ export async function fetchRange({
           cachedUrls,
           metrics,
           cachedUrlLookupEnabled,
+          canTryFromCache,
           fallbackToCacheURL,
           file,
         })