Merge pull request #8886 from overleaf/jpa-pdf-caching-alpha

[web] prepare alpha release of pdf caching

GitOrigin-RevId: 5617dd443da57b7077db793c2bc39be35ec44ef1

services/web/app/src/Features/Compile/CompileController.js

@@ -1,4 +1,5 @@
 let CompileController
+const { URL } = require('url')
 const OError = require('@overleaf/o-error')
 const Metrics = require('@overleaf/metrics')
 const ProjectGetter = require('../Project/ProjectGetter')
@@ -14,6 +15,7 @@ const ClsiCookieManager = require('./ClsiCookieManager')(
 )
 const Path = require('path')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
+const SplitTestHandler = require('../SplitTests/SplitTestHandler')
 
 const COMPILE_TIMEOUT_MS = 10 * 60 * 1000
 
@@ -25,18 +27,44 @@ function getImageNameForProject(projectId, callback) {
   })
 }
 
+function getEnablePdfCaching(req, res, cb) {
+  if (!req.query.enable_pdf_caching) {
+    // The frontend does not want to do pdf caching.
+    return cb(null, false)
+  }
+
+  // Use the query flags from the editor request for overriding the split test.
+  let query = {}
+  try {
+    const u = new URL(req.headers.referer || req.url, Settings.siteUrl)
+    query = Object.fromEntries(u.searchParams.entries())
+  } catch (e) {}
+  const editorReq = { ...req, query }
+
+  // Double check with the latest split test assignment.
+  // We may need to turn off the feature on a short notice, without requiring
+  //  all users to reload their editor page to disable the feature.
+  SplitTestHandler.getAssignment(
+    editorReq,
+    res,
+    'pdf-caching-mode',
+    (err, assignment) => {
+      if (err) return cb(null, false)
+      cb(null, assignment?.variant === 'enabled')
+    }
+  )
+}
+
 module.exports = CompileController = {
   compile(req, res, next) {
     res.setTimeout(COMPILE_TIMEOUT_MS)
     const projectId = req.params.Project_id
     const isAutoCompile = !!req.query.auto_compile
-    const enablePdfCaching = !!req.query.enable_pdf_caching
     const fileLineErrors = !!req.query.file_line_errors
     const stopOnFirstError = !!req.body.stopOnFirstError
     const userId = SessionManager.getLoggedInUserId(req.session)
     const options = {
       isAutoCompile,
-      enablePdfCaching,
       fileLineErrors,
       stopOnFirstError,
     }
@@ -63,63 +91,68 @@ module.exports = CompileController = {
       options.incrementalCompilesEnabled = true
     }
 
-    CompileManager.compile(
-      projectId,
-      userId,
-      options,
-      (
-        error,
-        status,
-        outputFiles,
-        clsiServerId,
-        limits,
-        validationProblems,
-        stats,
-        timings,
-        outputUrlPrefix
-      ) => {
-        if (error) {
-          Metrics.inc('compile-error')
-          return next(error)
-        }
-        Metrics.inc('compile-status', 1, { status })
-        let pdfDownloadDomain = Settings.pdfDownloadDomain
-        if (pdfDownloadDomain && outputUrlPrefix) {
-          pdfDownloadDomain += outputUrlPrefix
-        }
+    getEnablePdfCaching(req, res, (err, enablePdfCaching) => {
+      if (err) return next(err)
+      options.enablePdfCaching = enablePdfCaching
 
-        if (limits) {
-          // For a compile request to be sent to clsi we need limits.
-          // If we get here without having the limits object populated, it is
-          //  a reasonable assumption to make that nothing was compiled.
-          // We need to know the limits in order to make use of the events.
-          AnalyticsManager.recordEventForSession(
-            req.session,
-            'compile-result-backend',
-            {
-              projectId,
-              ownerAnalyticsId: limits.ownerAnalyticsId,
-              status,
-              compileTime: timings?.compileE2E,
-              timeout: limits.timeout === 60 ? 'short' : 'long',
-              server: clsiServerId?.includes('-c2d-') ? 'faster' : 'normal',
-              isAutoCompile,
-              stopOnFirstError,
-            }
-          )
-        }
-        res.json({
+      CompileManager.compile(
+        projectId,
+        userId,
+        options,
+        (
+          error,
           status,
           outputFiles,
-          compileGroup: limits?.compileGroup,
           clsiServerId,
+          limits,
           validationProblems,
           stats,
           timings,
-          pdfDownloadDomain,
-        })
-      }
-    )
+          outputUrlPrefix
+        ) => {
+          if (error) {
+            Metrics.inc('compile-error')
+            return next(error)
+          }
+          Metrics.inc('compile-status', 1, { status })
+          let pdfDownloadDomain = Settings.pdfDownloadDomain
+          if (pdfDownloadDomain && outputUrlPrefix) {
+            pdfDownloadDomain += outputUrlPrefix
+          }
+
+          if (limits) {
+            // For a compile request to be sent to clsi we need limits.
+            // If we get here without having the limits object populated, it is
+            //  a reasonable assumption to make that nothing was compiled.
+            // We need to know the limits in order to make use of the events.
+            AnalyticsManager.recordEventForSession(
+              req.session,
+              'compile-result-backend',
+              {
+                projectId,
+                ownerAnalyticsId: limits.ownerAnalyticsId,
+                status,
+                compileTime: timings?.compileE2E,
+                timeout: limits.timeout === 60 ? 'short' : 'long',
+                server: clsiServerId?.includes('-c2d-') ? 'faster' : 'normal',
+                isAutoCompile,
+                stopOnFirstError,
+              }
+            )
+          }
+          res.json({
+            status,
+            outputFiles,
+            compileGroup: limits?.compileGroup,
+            clsiServerId,
+            validationProblems,
+            stats,
+            timings,
+            pdfDownloadDomain,
+          })
+        }
+      )
+    })
   },
 
   stopCompile(req, res, next) {

services/web/app/src/Features/Project/ProjectController.js

@@ -983,6 +983,18 @@ const ProjectController = {
             }
           )
         },
+        trackPdfDownloadAssignment(cb) {
+          SplitTestHandler.getAssignment(req, res, 'track-pdf-download', () => {
+            // We'll pick up the assignment from the res.locals assignment.
+            cb()
+          })
+        },
+        pdfCachingModeAssignment(cb) {
+          SplitTestHandler.getAssignment(req, res, 'pdf-caching-mode', () => {
+            // We'll pick up the assignment from the res.locals assignment.
+            cb()
+          })
+        },
       },
       (
         err,
@@ -1071,30 +1083,6 @@ const ProjectController = {
               }
             }
 
-            function getTrackPdfDownload() {
-              if (!Settings.enablePdfCaching) {
-                // The feature is disabled globally.
-                return false
-              }
-              // Let the user opt-in only.
-              // The flag will opt-out of both caching and metrics collection,
-              //  as if this editing session never happened.
-              return shouldDisplayFeature('track-pdf-download', false)
-            }
-
-            function getPdfCachingMode() {
-              if (!Settings.enablePdfCaching) {
-                // The feature is disabled globally.
-                return ''
-              }
-              // Let the user opt-in only.
-              const v = req.query['pdf-caching-mode']
-              if (['enabled'].includes(v)) {
-                return v
-              }
-              return ''
-            }
-
             const showPdfDetach = shouldDisplayFeature(
               'pdf_detach',
               pdfDetachAssignment.variant === 'enabled'
@@ -1199,8 +1187,6 @@ const ProjectController = {
               showNewSourceEditorOption,
               showSymbolPalette,
               showStopOnFirstError,
-              trackPdfDownload: getTrackPdfDownload(),
-              pdfCachingMode: getPdfCachingMode(),
               detachRole,
               metadata: { viewport: false },
               showHeaderUpgradePrompt,

services/web/app/views/project/editor/meta.pug

@@ -26,8 +26,6 @@ meta(name="ol-showPdfDetach" data-type="boolean" content=showPdfDetach)
 meta(name="ol-debugPdfDetach" data-type="boolean" content=debugPdfDetach)
 meta(name="ol-showNewSourceEditorOption" data-type="boolean" content=showNewSourceEditorOption)
 meta(name="ol-showSymbolPalette" data-type="boolean" content=showSymbolPalette)
-meta(name="ol-pdfCachingMode" content=pdfCachingMode)
-meta(name="ol-trackPdfDownload" data-type="boolean" content=trackPdfDownload)
 meta(name="ol-detachRole" data-type="string" content=detachRole)
 meta(name="ol-showHeaderUpgradePrompt" data-type="boolean" content=showHeaderUpgradePrompt)
 meta(name="ol-showStopOnFirstError" data-type="boolean" content=showStopOnFirstError)

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

@@ -3,6 +3,7 @@ import getMeta from '../../../utils/meta'
 import { deleteJSON, postJSON } from '../../../infrastructure/fetch-json'
 import { debounce } from 'lodash'
 import { trackPdfDownload } from './metrics'
+import { enablePdfCaching } from './pdf-caching-flags'
 
 const AUTO_COMPILE_MAX_WAIT = 5000
 // We add a 2 second debounce to sending user changes to server if they aren't
@@ -168,7 +169,7 @@ export default class DocumentCompiler {
     }
 
     // use the feature flag to enable PDF caching
-    if (getMeta('ol-pdfCachingMode') === 'enabled') {
+    if (enablePdfCaching) {
       params.set('enable_pdf_caching', 'true')
     }
 

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

@@ -1,6 +1,6 @@
 import { v4 as uuid } from 'uuid'
 import { sendMB } from '../../../infrastructure/event-tracking'
-import getMeta from '../../../utils/meta'
+import { trackPdfDownloadEnabled } from './pdf-caching-flags'
 
 // VERSION should get incremented when making changes to caching behavior or
 //  adjusting metrics collection.
@@ -39,7 +39,7 @@ export function trackPdfDownload(response, compileTimeClientE2E, t0) {
     if (latencyRender) {
       deliveryLatencies.latencyRender = latencyRender
     }
-    if (getMeta('ol-trackPdfDownload')) {
+    if (trackPdfDownloadEnabled) {
       // Submit latency along with compile context.
       submitCompileMetrics({
         totalDeliveryTime,

services/web/frontend/js/features/pdf-preview/util/output-files.js

@@ -2,6 +2,7 @@ import getMeta from '../../../utils/meta'
 import HumanReadableLogs from '../../../ide/human-readable-logs/HumanReadableLogs'
 import BibLogParser from '../../../ide/log-parser/bib-log-parser'
 import { v4 as uuid } from 'uuid'
+import { enablePdfCaching } from './pdf-caching-flags'
 
 // Warnings that may disappear after a second LaTeX pass
 const TRANSIENT_WARNING_REGEX = /^(Reference|Citation).+undefined on input line/
@@ -19,7 +20,7 @@ export function handleOutputFiles(outputFiles, projectId, data) {
     params.set('clsiserverid', data.clsiServerId)
   }
 
-  if (getMeta('ol-pdfCachingMode') === 'enabled') {
+  if (enablePdfCaching) {
     // Tag traffic that uses the pdf caching logic.
     params.set('enable_pdf_caching', 'true')
   }

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

@@ -0,0 +1,8 @@
+import getMeta from '../../../utils/meta'
+
+function isFlagEnabled(flag) {
+  return getMeta('ol-splitTestVariants')?.[flag] === 'enabled'
+}
+
+export const enablePdfCaching = isFlagEnabled('pdf-caching-mode')
+export const trackPdfDownloadEnabled = isFlagEnabled('track-pdf-download')

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

@@ -1,10 +1,11 @@
 import { fallbackRequest, fetchRange } from './pdf-caching'
-import getMeta from '../../../utils/meta'
 import { captureException } from '../../../infrastructure/error-reporter'
 import { getPdfCachingMetrics } from './metrics'
+import { enablePdfCaching, trackPdfDownloadEnabled } from './pdf-caching-flags'
 
 export function generatePdfCachingTransportFactory(PDFJS) {
-  if (getMeta('ol-pdfCachingMode') !== 'enabled') {
+  // NOTE: The custom transport can be used for tracking download volume.
+  if (!enablePdfCaching && !trackPdfDownloadEnabled) {
     return () => null
   }
   let failedOnce = false
@@ -19,6 +20,7 @@ export function generatePdfCachingTransportFactory(PDFJS) {
     fetchedBytes: 0,
     requestedCount: 0,
     requestedBytes: 0,
+    enablePdfCaching,
   })
   const verifyChunks =
     new URLSearchParams(window.location.search).get('verify_chunks') === 'true'
@@ -51,6 +53,9 @@ export function generatePdfCachingTransportFactory(PDFJS) {
         .catch(err => {
           metrics.failedCount++
           failedOnce = true
+          if (!enablePdfCaching) {
+            throw err // This was a fallback request already. Do not retry.
+          }
           console.error('optimized pdf download error', err)
           captureException(err)
           return fallbackRequest({ url: this.url, start, end, abortSignal })

services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -1126,77 +1126,6 @@ describe('ProjectController', function () {
       this.ProjectController.loadEditor(this.req, this.res)
     })
 
-    describe('pdf caching feature flags', function () {
-      function expectBandwidthTrackingEnabled() {
-        it('should track pdf bandwidth', function (done) {
-          this.res.render = (pageName, opts) => {
-            expect(opts.trackPdfDownload).to.equal(true)
-            done()
-          }
-          this.ProjectController.loadEditor(this.req, this.res)
-        })
-      }
-      function expectPDFCachingEnabled(mode) {
-        it('should enable pdf caching', function (done) {
-          this.res.render = (pageName, opts) => {
-            expect(opts.pdfCachingMode).to.equal(mode)
-            done()
-          }
-          this.ProjectController.loadEditor(this.req, this.res)
-        })
-      }
-      function expectBandwidthTrackingDisabled() {
-        it('should not track pdf bandwidth', function (done) {
-          this.res.render = (pageName, opts) => {
-            expect(opts.trackPdfDownload).to.equal(false)
-            done()
-          }
-          this.ProjectController.loadEditor(this.req, this.res)
-        })
-      }
-      function expectPDFCachingDisabled() {
-        it('should disable pdf caching', function (done) {
-          this.res.render = (pageName, opts) => {
-            expect(opts.pdfCachingMode).to.equal('')
-            done()
-          }
-          this.ProjectController.loadEditor(this.req, this.res)
-        })
-      }
-
-      beforeEach(function () {
-        this.settings.enablePdfCaching = true
-      })
-
-      describe('during opt-in only', function () {
-        describe('with no query', function () {
-          expectBandwidthTrackingDisabled()
-          expectPDFCachingDisabled()
-        })
-
-        describe('with track-pdf-download=false', function () {
-          beforeEach(function () {
-            this.req.query['track-pdf-download'] = 'false'
-          })
-          expectBandwidthTrackingDisabled()
-        })
-
-        describe('with track-pdf-download=true', function () {
-          beforeEach(function () {
-            this.req.query['track-pdf-download'] = 'true'
-          })
-          expectBandwidthTrackingEnabled()
-        })
-
-        describe('with pdf-caching-mode=enabled', function () {
-          beforeEach(function () {
-            this.req.query['pdf-caching-mode'] = 'enabled'
-          })
-          expectPDFCachingEnabled('enabled')
-        })
-      })
-    })
-
     describe('wsUrl', function () {
       function checkLoadEditorWsMetric(metric) {
         it(`should inc metric ${metric}`, function (done) {