Просмотр исходного кода

Merge pull request #12198 from overleaf/jpa-force-new-compile-domain

[web] changes for force-new-compile-domain test

GitOrigin-RevId: a4ceaf46fdcebed156d155385cbbc2f06405d31f
Jakob Ackermann 3 лет назад
Родитель
Сommit
df3c7e48ab

+ 13 - 1
services/web/app/src/Features/Compile/CompileController.js

@@ -58,8 +58,15 @@ const getSplitTestOptions = callbackify(async function (req, res) {
       res,
       'pdf-download-domain'
     )
+  const { variant: forceNewDomainVariant } =
+    await SplitTestHandler.promises.getAssignment(
+      editorReq,
+      res,
+      'force-new-compile-domain'
+    )
   const pdfDownloadDomain =
-    domainVariant === 'user' && Settings.compilesUserContentDomain
+    (domainVariant === 'user' || forceNewDomainVariant === 'enabled') &&
+    Settings.compilesUserContentDomain
       ? Settings.compilesUserContentDomain
       : Settings.pdfDownloadDomain
 
@@ -77,6 +84,7 @@ const getSplitTestOptions = callbackify(async function (req, res) {
       pdfDownloadDomain,
       enableHybridPdfDownload,
       enablePdfCaching: false,
+      forceNewDomainVariant,
     }
   }
 
@@ -95,6 +103,7 @@ const getSplitTestOptions = callbackify(async function (req, res) {
       pdfDownloadDomain,
       enableHybridPdfDownload,
       enablePdfCaching: false,
+      forceNewDomainVariant,
     }
   }
   const pdfCachingMinChunkSize = await getPdfCachingMinChunkSize(editorReq, res)
@@ -103,6 +112,7 @@ const getSplitTestOptions = callbackify(async function (req, res) {
     enableHybridPdfDownload,
     enablePdfCaching,
     pdfCachingMinChunkSize,
+    forceNewDomainVariant,
   }
 })
 
@@ -149,6 +159,7 @@ module.exports = CompileController = {
         pdfCachingMinChunkSize,
         pdfDownloadDomain,
         enableHybridPdfDownload,
+        forceNewDomainVariant,
       } = splitTestOptions
       options.enablePdfCaching = enablePdfCaching
       if (enablePdfCaching) {
@@ -217,6 +228,7 @@ module.exports = CompileController = {
             pdfDownloadDomain,
             pdfCachingMinChunkSize,
             enableHybridPdfDownload,
+            forceNewDomainVariant,
           })
         }
       )

+ 11 - 0
services/web/app/src/Features/Project/ProjectController.js

@@ -1064,6 +1064,17 @@ const ProjectController = {
             }
           )
         },
+        forceNewDomainAssignment(cb) {
+          SplitTestHandler.getAssignment(
+            req,
+            res,
+            'force-new-compile-domain',
+            () => {
+              // We'll pick up the assignment from the res.locals assignment.
+              cb()
+            }
+          )
+        },
         userContentDomainAccessCheckAssigment(cb) {
           SplitTestHandler.getAssignment(
             req,

+ 1 - 0
services/web/frontend/extracted-translations.json

@@ -499,6 +499,7 @@
   "need_to_leave": "",
   "need_to_upgrade_for_more_collabs": "",
   "need_to_upgrade_for_more_collabs_variant": "",
+  "new_compile_domain_trouble_shooting": "",
   "new_file": "",
   "new_folder": "",
   "new_name": "",

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

@@ -11,6 +11,10 @@ import PdfPreviewErrorBoundaryFallback from './pdf-preview-error-boundary-fallba
 import { useDetachCompileContext as useCompileContext } from '../../../shared/context/detach-compile-context'
 import { captureException } from '../../../infrastructure/error-reporter'
 import { getPdfCachingMetrics } from '../util/metrics'
+import { userContentDomainAccessCheckFailed } from '../../user-content-domain-access-check'
+import { isURLOnUserContentDomain } from '../util/fetchFromCompileDomain'
+import { isNetworkError } from '../../../utils/isNetworkError'
+import OError from '@overleaf/o-error'
 
 function PdfJsViewer({ url, pdfFile }) {
   const { _id: projectId } = useProjectContext()
@@ -126,7 +130,15 @@ function PdfJsViewer({ url, pdfFile }) {
         if (abortController.signal.aborted) return
         // The error is already logged at the call-site with additional context.
         if (err instanceof pdfJsWrapper.PDFJS.MissingPDFException) {
-          setError('rendering-error-expected')
+          if (
+            // 404 is unrelated to new domain
+            OError.getFullInfo(err).statusCode !== 404 &&
+            isURLOnUserContentDomain(OError.getFullInfo(err).url)
+          ) {
+            setError('rendering-error-new-domain')
+          } else {
+            setError('rendering-error-expected')
+          }
         } else {
           setError('rendering-error')
         }
@@ -136,7 +148,22 @@ function PdfJsViewer({ url, pdfFile }) {
         .catch(error => {
           if (abortController.signal.aborted) return
           console.error(error)
-          setError('rendering-error')
+          if (
+            isURLOnUserContentDomain(url) &&
+            error instanceof pdfJsWrapper.PDFJS.UnexpectedResponseException
+          ) {
+            setError('rendering-error-new-domain')
+          } else if (
+            isURLOnUserContentDomain(url) &&
+            error.name === 'UnknownErrorException' &&
+            (isNetworkError(error) || userContentDomainAccessCheckFailed())
+          ) {
+            // For some reason, pdfJsWrapper.PDFJS.UnknownErrorException is
+            //  not available for an instance check.
+            setError('rendering-error-new-domain')
+          } else {
+            setError('rendering-error')
+          }
         })
       return () => {
         abortController.abort()

+ 27 - 0
services/web/frontend/js/features/pdf-preview/components/pdf-preview-error.js

@@ -5,6 +5,7 @@ import { Button } from 'react-bootstrap'
 import PdfLogEntry from './pdf-log-entry'
 import { useDetachCompileContext as useCompileContext } from '../../../shared/context/detach-compile-context'
 import { useStopOnFirstError } from '../../../shared/hooks/use-stop-on-first-error'
+import getMeta from '../../../utils/meta'
 
 function PdfPreviewError({ error }) {
   const { t } = useTranslation()
@@ -12,6 +13,32 @@ function PdfPreviewError({ error }) {
   const { startCompile } = useCompileContext()
 
   switch (error) {
+    case 'rendering-error-new-domain':
+      return (
+        <PdfLogEntry
+          headerTitle={t('pdf_rendering_error')}
+          formattedContent={
+            <Trans
+              i18nKey="new_compile_domain_trouble_shooting"
+              values={{
+                compilesUserContentDomain: new URL(
+                  getMeta('ol-compilesUserContentDomain')
+                ).hostname,
+              }}
+              components={[
+                <code key="domain" />,
+                /* eslint-disable-next-line jsx-a11y/anchor-has-content */
+                <a
+                  href="/learn/how-to/Resolving_access%2C_loading%2C_and_display_problems"
+                  target="_blank"
+                  key="troubleshooting-link"
+                />,
+              ]}
+            />
+          }
+          level="warning"
+        />
+      )
     case 'rendering-error-expected':
       return (
         <PdfLogEntry

+ 17 - 7
services/web/frontend/js/features/pdf-preview/util/fetchFromCompileDomain.ts

@@ -2,13 +2,14 @@ import { isNetworkError } from '../../../utils/isNetworkError'
 import getMeta from '../../../utils/meta'
 import OError from '@overleaf/o-error'
 import { postJSON } from '../../../infrastructure/fetch-json'
+import { isSplitTestEnabled } from '../../../utils/splitTestUtils'
 
 let useFallbackDomainUntil = performance.now()
 const ONE_HOUR_IN_MS = 1000 * 60 * 60
 
 class MaybeBlockedByProxyError extends OError {}
 
-function checkForBlockingByProxy(res: Response) {
+function checkForBlockingByProxy(url: string, res: Response) {
   const statusCode = res.status
   switch (statusCode) {
     case 200: // full response
@@ -19,18 +20,26 @@ function checkForBlockingByProxy(res: Response) {
     default:
       throw new MaybeBlockedByProxyError('request might be blocked by proxy', {
         res,
+        url,
         statusCode,
       })
   }
 }
 
-export async function fetchFromCompileDomain(url: string, init: RequestInit) {
+export function isURLOnUserContentDomain(url: string) {
   const userContentDomain = getMeta('ol-compilesUserContentDomain')
-  let isUserContentDomain =
+  return (
     userContentDomain &&
+    url &&
     new URL(url).hostname === new URL(userContentDomain).hostname
+  )
+}
+
+export async function fetchFromCompileDomain(url: string, init: RequestInit) {
+  let isUserContentDomain = isURLOnUserContentDomain(url)
+  const fallbackAllowed = !isSplitTestEnabled('force-new-compile-domain')
 
-  if (useFallbackDomainUntil > performance.now()) {
+  if (fallbackAllowed && useFallbackDomainUntil > performance.now()) {
     isUserContentDomain = false
     url = withFallbackCompileDomain(url)
   }
@@ -39,13 +48,14 @@ export async function fetchFromCompileDomain(url: string, init: RequestInit) {
     if (isUserContentDomain) {
       // Only throw a MaybeBlockedByProxyError when the request will be retried
       //  on the fallback domain below.
-      checkForBlockingByProxy(res)
+      checkForBlockingByProxy(url, res)
     }
     return res
   } catch (err) {
     if (
-      (isNetworkError(err) || err instanceof MaybeBlockedByProxyError) &&
-      isUserContentDomain
+      fallbackAllowed &&
+      isUserContentDomain &&
+      (isNetworkError(err) || err instanceof MaybeBlockedByProxyError)
     ) {
       try {
         const res = await fetch(withFallbackCompileDomain(url), init)

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

@@ -10,6 +10,8 @@ import {
   trackPdfDownloadEnabled,
 } from './pdf-caching-flags'
 import { isNetworkError } from '../../../utils/isNetworkError'
+import { isSplitTestEnabled } from '../../../utils/splitTestUtils'
+import { isURLOnUserContentDomain } from './fetchFromCompileDomain'
 
 // 30 seconds: The shutdown grace period of a clsi pre-emp instance.
 const STALE_OUTPUT_REQUEST_THRESHOLD_MS = 30 * 1000
@@ -76,10 +78,13 @@ export function generatePdfCachingTransportFactory(PDFJS) {
         end,
         metrics,
       })
+      const isExpectedFailureOnNewCompileDomain = err =>
+        isSplitTestEnabled('force-new-compile-domain') &&
+        isURLOnUserContentDomain(OError.getFullInfo(err).url)
 
       const isStaleOutputRequest = () =>
         performance.now() - this.startTime > STALE_OUTPUT_REQUEST_THRESHOLD_MS
-      const is404 = err => err.message === 'non successful response status: 404'
+      const is404 = err => OError.getFullInfo(err).statusCode === 404
       const isFromOutputPDFRequest = err =>
         OError.getFullInfo(err).url === this.url
 
@@ -91,8 +96,9 @@ export function generatePdfCachingTransportFactory(PDFJS) {
       // - requests for the main output.pdf file
       //   A fallback request would not be able to retrieve the PDF either.
       const isExpectedError = err =>
-        (is404(err) || isNetworkError(err)) &&
-        (isStaleOutputRequest() || isFromOutputPDFRequest(err))
+        ((is404(err) || isNetworkError(err)) &&
+          (isStaleOutputRequest() || isFromOutputPDFRequest(err))) ||
+        isExpectedFailureOnNewCompileDomain(err)
 
       fetchRange({
         url: this.url,
@@ -113,12 +119,17 @@ export function generatePdfCachingTransportFactory(PDFJS) {
           if (isExpectedError(err)) {
             if (is404(err)) {
               // A regular pdf-js request would have seen this 404 as well.
+            } else if (isExpectedFailureOnNewCompileDomain(err)) {
+              // A regular pdf-js request would have seen this proxy-error as well.
             } else {
               // Flaky network, switch back to regular pdf-js requests.
               metrics.failedCount++
               metrics.failedOnce = true
             }
-            throw new PDFJS.MissingPDFException()
+            throw OError.tag(new PDFJS.MissingPDFException(), 'caching', {
+              statusCode: OError.getFullInfo(err).statusCode,
+              url: OError.getFullInfo(err).url,
+            })
           }
           metrics.failedCount++
           metrics.failedOnce = true
@@ -140,7 +151,10 @@ export function generatePdfCachingTransportFactory(PDFJS) {
             abortSignal,
           }).catch(err => {
             if (isExpectedError(err)) {
-              err = new PDFJS.MissingPDFException()
+              throw OError.tag(new PDFJS.MissingPDFException(), 'fallback', {
+                statusCode: OError.getFullInfo(err).statusCode,
+                url: OError.getFullInfo(err).url,
+              })
             }
             throw err
           })

+ 1 - 0
services/web/frontend/js/features/pdf-preview/util/pdf-caching.js

@@ -449,6 +449,7 @@ export function resolveMultiPartResponses({
 export function checkChunkResponse(response, estimatedSize, init) {
   if (!(response.status === 206 || response.status === 200)) {
     throw new OError('non successful response status: ' + response.status, {
+      statusCode: response.status,
       responseHeaders: Object.fromEntries(response.headers.entries()),
       requestHeader: init.headers,
     })

+ 13 - 7
services/web/frontend/js/features/user-content-domain-access-check/index.ts

@@ -239,22 +239,28 @@ export async function checkUserContentDomainAccess(
   return failed === 0
 }
 
-let accessCheckPassed = false
+const ACCESS_CHECK_PASSED = 'passed'
+const ACCESS_CHECK_PENDING = 'pending'
+const ACCESS_CHECK_FAILED = 'failed'
+let accessCheckStatus = ACCESS_CHECK_PENDING
 
 export function userContentDomainAccessCheckPassed() {
-  return accessCheckPassed
+  return accessCheckStatus === ACCESS_CHECK_PASSED
+}
+export function userContentDomainAccessCheckFailed() {
+  return accessCheckStatus === ACCESS_CHECK_FAILED
 }
 
 let networkEpoch = performance.now()
 window.addEventListener('offline', () => {
   // We are offline. Abort any scheduled check.
   clearTimeout(lastScheduledCheck)
-  accessCheckPassed = false
+  accessCheckStatus = ACCESS_CHECK_PENDING
   networkEpoch = performance.now()
 })
 window.addEventListener('online', () => {
   // We are online again. Schedule another check for this network.
-  accessCheckPassed = false
+  accessCheckStatus = ACCESS_CHECK_PENDING
   networkEpoch = performance.now()
   scheduleUserContentDomainAccessCheck()
 })
@@ -263,7 +269,7 @@ try {
   // Docs: https://developer.mozilla.org/en-US/docs/Web/API/NetworkInformation
   navigator.connection.addEventListener('change', () => {
     // The network changed. Schedule another check for it.
-    accessCheckPassed = false
+    accessCheckStatus = ACCESS_CHECK_PENDING
     networkEpoch = performance.now()
     scheduleUserContentDomainAccessCheck()
   })
@@ -282,7 +288,7 @@ export function scheduleUserContentDomainAccessCheck() {
       // Try again in INITIAL_DELAY_MS.
       return scheduleUserContentDomainAccessCheck()
     }
-    if (accessCheckPassed) return
+    if (userContentDomainAccessCheckPassed()) return
     if (remainingChecks === 0) {
       recordMaxAccessChecksHit()
     }
@@ -294,7 +300,7 @@ export function scheduleUserContentDomainAccessCheck() {
     }
     checkUserContentDomainAccess(getMeta('ol-compilesUserContentDomain'))
       .then(ok => {
-        accessCheckPassed = ok
+        accessCheckStatus = ok ? ACCESS_CHECK_PASSED : ACCESS_CHECK_FAILED
       })
       .catch(err => {
         captureException(err)

+ 16 - 1
services/web/frontend/js/shared/context/detach-compile-context.js

@@ -1,4 +1,4 @@
-import { createContext, useContext, useMemo } from 'react'
+import { createContext, useContext, useEffect, useMemo } from 'react'
 import PropTypes from 'prop-types'
 import {
   useLocalCompileContext,
@@ -7,6 +7,7 @@ import {
 import useDetachStateWatcher from '../hooks/use-detach-state-watcher'
 import useDetachAction from '../hooks/use-detach-action'
 import useCompileTriggers from '../../features/pdf-preview/hooks/use-compile-triggers'
+import getMeta from '../../utils/meta'
 
 export const DetachCompileContext = createContext()
 
@@ -31,6 +32,7 @@ export function DetachCompileProvider({ children }) {
     draft: _draft,
     error: _error,
     fileList: _fileList,
+    forceNewDomainVariant: _forceNewDomainVariant,
     hasChanges: _hasChanges,
     highlights: _highlights,
     lastCompileOptions: _lastCompileOptions,
@@ -120,6 +122,12 @@ export function DetachCompileProvider({ children }) {
     'detacher',
     'detached'
   )
+  const [forceNewDomainVariant] = useDetachStateWatcher(
+    'forceNewDomainVariant',
+    _forceNewDomainVariant,
+    'detacher',
+    'detached'
+  )
   const [hasChanges] = useDetachStateWatcher(
     'hasChanges',
     _hasChanges,
@@ -345,6 +353,11 @@ export function DetachCompileProvider({ children }) {
   )
 
   useCompileTriggers(startCompile, setChangedAt)
+  useEffect(() => {
+    // Sync the split test variant across the editor and pdf-detach.
+    const variants = getMeta('ol-splitTestVariants') || {}
+    variants['force-new-compile-domain'] = forceNewDomainVariant
+  }, [forceNewDomainVariant])
 
   const value = useMemo(
     () => ({
@@ -359,6 +372,7 @@ export function DetachCompileProvider({ children }) {
       draft,
       error,
       fileList,
+      forceNewDomainVariant,
       hasChanges,
       highlights,
       lastCompileOptions,
@@ -410,6 +424,7 @@ export function DetachCompileProvider({ children }) {
       draft,
       error,
       fileList,
+      forceNewDomainVariant,
       hasChanges,
       highlights,
       lastCompileOptions,

+ 10 - 0
services/web/frontend/js/shared/context/local-compile-context.js

@@ -29,6 +29,7 @@ import { useEditorContext } from './editor-context'
 import { buildFileList } from '../../features/pdf-preview/util/file-list'
 import { useLayoutContext } from './layout-context'
 import { useUserContext } from './user-context'
+import getMeta from '../../utils/meta'
 
 export const LocalCompileContext = createContext()
 
@@ -43,6 +44,7 @@ export const CompileContextPropTypes = {
     draft: PropTypes.bool.isRequired,
     error: PropTypes.string,
     fileList: PropTypes.object,
+    forceNewDomainVariant: PropTypes.string,
     hasChanges: PropTypes.bool.isRequired,
     highlights: PropTypes.arrayOf(PropTypes.object),
     logEntries: PropTypes.object,
@@ -168,6 +170,11 @@ export function LocalCompileProvider({ children }) {
   // the list of files that can be downloaded
   const [fileList, setFileList] = useState()
 
+  // Split test variant for disabling the fallback, refreshed on re-compile.
+  const [forceNewDomainVariant, setForceNewDomainVariant] = useState(
+    getMeta('ol-splitTestVariants')?.['force-new-compile-domain']
+  )
+
   // the raw contents of the log file
   const [rawLog, setRawLog] = useState()
 
@@ -305,6 +312,7 @@ export function LocalCompileProvider({ children }) {
       setShowFasterCompilesFeedbackUI(
         Boolean(data.showFasterCompilesFeedbackUI)
       )
+      setForceNewDomainVariant(data.forceNewDomainVariant || 'default')
 
       if (data.outputFiles) {
         const outputFiles = new Map()
@@ -527,6 +535,7 @@ export function LocalCompileProvider({ children }) {
       draft,
       error,
       fileList,
+      forceNewDomainVariant,
       hasChanges,
       highlights,
       lastCompileOptions,
@@ -578,6 +587,7 @@ export function LocalCompileProvider({ children }) {
       draft,
       error,
       fileList,
+      forceNewDomainVariant,
       hasChanges,
       highlights,
       lastCompileOptions,

+ 1 - 0
services/web/locales/en.json

@@ -941,6 +941,7 @@
   "need_to_leave": "Need to leave?",
   "need_to_upgrade_for_more_collabs": "You need to upgrade your account to add more collaborators",
   "need_to_upgrade_for_more_collabs_variant": "You have reached the maximum number of collaborators. Upgrade your account to add more.",
+  "new_compile_domain_trouble_shooting": "We are migrating PDF downloads to a new domain. It looks like something is blocking your browser from accessing that new domain, <0>__compilesUserContentDomain__</0>. This could be caused by network blocking or a strict browser plugin rule. Please follow our <1>troubleshooting guide</1>.",
   "new_file": "New File",
   "new_folder": "New Folder",
   "new_name": "New Name",

+ 3 - 0
services/web/test/unit/src/Compile/CompileControllerTests.js

@@ -127,6 +127,7 @@ describe('CompileController', function () {
               ],
               pdfDownloadDomain: 'https://compiles.overleaf.test',
               enableHybridPdfDownload: false,
+              forceNewDomainVariant: 'default',
             })
           )
         })
@@ -170,6 +171,7 @@ describe('CompileController', function () {
               ],
               pdfDownloadDomain: 'https://compiles.overleaf.test/zone/b',
               enableHybridPdfDownload: false,
+              forceNewDomainVariant: 'default',
             })
           )
         })
@@ -212,6 +214,7 @@ describe('CompileController', function () {
             status: this.status,
             outputFiles: this.outputFiles,
             enableHybridPdfDownload: false,
+            forceNewDomainVariant: 'default',
           })
         )
       })