Merge pull request #9058 from overleaf/jpa-error-handling-2nd-page

[web] improve error handling for pdf-caching

GitOrigin-RevId: ddd65f2813fb08eb4493967071ee62d9742930cc

services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js

@@ -127,11 +127,23 @@ function PdfJsViewer({ url, pdfFile }) {
       setError(undefined)
       setStartFetch(performance.now())
 
-      pdfJsWrapper.loadDocument(url, pdfFile).catch(error => {
-        console.error(error)
+      const abortController = new AbortController()
+      const handleFetchError = () => {
+        if (abortController.signal.aborted) return
+        // The error is already logged at the call-site with additional context.
         setError('rendering-error')
-      })
-      return () => pdfJsWrapper.abortDocumentLoading()
+      }
+      pdfJsWrapper
+        .loadDocument({ url, pdfFile, abortController, handleFetchError })
+        .catch(error => {
+          if (abortController.signal.aborted) return
+          console.error(error)
+          setError('rendering-error')
+        })
+      return () => {
+        abortController.abort()
+        pdfJsWrapper.abortDocumentLoading()
+      }
     }
   }, [pdfJsWrapper, url, pdfFile, setError, setStartFetch])
 

services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.js

@@ -9,10 +9,10 @@ export function generatePdfCachingTransportFactory(PDFJS) {
   if (!enablePdfCaching && !trackPdfDownloadEnabled) {
     return () => null
   }
-  let failedOnce = false
   const cached = new Set()
   const metrics = Object.assign(getPdfCachingMetrics(), {
     failedCount: 0,
+    failedOnce: false,
     tooLargeOverheadCount: 0,
     tooManyRequestsCount: 0,
     cachedCount: 0,
@@ -27,12 +27,12 @@ export function generatePdfCachingTransportFactory(PDFJS) {
     new URLSearchParams(window.location.search).get('verify_chunks') === 'true'
 
   class PDFDataRangeTransport extends PDFJS.PDFDataRangeTransport {
-    constructor(url, pdfFile, reject) {
+    constructor({ url, pdfFile, abortController, handleFetchError }) {
       super(pdfFile.size, new Uint8Array())
       this.url = url
       this.pdfFile = pdfFile
-      this.reject = reject
-      this.abortController = new AbortController()
+      this.handleFetchError = handleFetchError
+      this.abortController = abortController
     }
 
     abort() {
@@ -59,17 +59,18 @@ export function generatePdfCachingTransportFactory(PDFJS) {
         abortSignal,
       })
         .catch(err => {
+          if (abortSignal.aborted) return
           if (
             err.message === 'non successful response status: 404' &&
             OError.getFullInfo(err).url === this.url
           ) {
             // Do not consider a 404 on the main pdf url as pdf caching failure.
             // Still, bail out during the initial launch phase.
-            failedOnce = true
+            metrics.failedOnce = true
             throw new PDFJS.MissingPDFException()
           }
           metrics.failedCount++
-          failedOnce = true
+          metrics.failedOnce = true
           if (!enablePdfCaching) {
             throw err // This was a fallback request already. Do not retry.
           }
@@ -79,25 +80,33 @@ export function generatePdfCachingTransportFactory(PDFJS) {
           return fallbackRequest({ url: this.url, start, end, abortSignal })
         })
         .then(blob => {
+          if (abortSignal.aborted) return
           this.onDataRange(start, blob)
         })
         .catch(err => {
+          if (abortSignal.aborted) return
           err = OError.tag(err, 'fatal pdf download error', errorInfo)
           console.error(err)
           if (!(err instanceof PDFJS.MissingPDFException)) {
             captureException(err, { tags: { fromPdfCaching: true } })
           }
-          this.reject(err)
+          // Signal error for (subsequent) page load.
+          this.handleFetchError(err)
         })
     }
   }
 
-  return function (url, pdfFile, reject) {
-    if (failedOnce) {
+  return function ({ url, pdfFile, abortController, handleFetchError }) {
+    if (metrics.failedOnce) {
       // Disable pdf caching once any fetch request failed.
       // Be trigger-happy here until we reached a stable state of the feature.
       return null
     }
-    return new PDFDataRangeTransport(url, pdfFile, reject)
+    return new PDFDataRangeTransport({
+      url,
+      pdfFile,
+      abortController,
+      handleFetchError,
+    })
   }
 }

services/web/frontend/js/features/pdf-preview/util/pdf-js-wrapper.js

@@ -64,7 +64,7 @@ export default class PDFJSWrapper {
   }
 
   // load a document from a URL
-  loadDocument(url, pdfFile) {
+  loadDocument({ url, pdfFile, abortController, handleFetchError }) {
     // cancel any previous loading task
     if (this.loadDocumentTask) {
       this.loadDocumentTask.destroy()
@@ -72,7 +72,12 @@ export default class PDFJSWrapper {
     }
 
     return new Promise((resolve, reject) => {
-      const rangeTransport = this.genPdfCachingTransport(url, pdfFile, reject)
+      const rangeTransport = this.genPdfCachingTransport({
+        url,
+        pdfFile,
+        abortController,
+        handleFetchError,
+      })
       let rangeChunkSize = DEFAULT_RANGE_CHUNK_SIZE
       if (rangeTransport && pdfFile.size < 2 * DEFAULT_RANGE_CHUNK_SIZE) {
         // pdf.js disables the "bulk" download optimization when providing a