diff --git a/services/web/app/src/Features/Compile/CompileController.js b/services/web/app/src/Features/Compile/CompileController.js index 5503a94741..aebd51d03e 100644 --- a/services/web/app/src/Features/Compile/CompileController.js +++ b/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, }) } ) diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 5aaa5025ab..facbd7ff3c 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/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, diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 7cd2537f9d..8381fd9fd4 100644 --- a/services/web/frontend/extracted-translations.json +++ b/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": "", diff --git a/services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js b/services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js index 3004d27db4..5ea09b9d38 100644 --- a/services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js +++ b/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() diff --git a/services/web/frontend/js/features/pdf-preview/components/pdf-preview-error.js b/services/web/frontend/js/features/pdf-preview/components/pdf-preview-error.js index ea96413752..3b1b5d238d 100644 --- a/services/web/frontend/js/features/pdf-preview/components/pdf-preview-error.js +++ b/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 ( + , + /* eslint-disable-next-line jsx-a11y/anchor-has-content */ + , + ]} + /> + } + level="warning" + /> + ) case 'rendering-error-expected': return ( performance.now()) { +export async function fetchFromCompileDomain(url: string, init: RequestInit) { + let isUserContentDomain = isURLOnUserContentDomain(url) + const fallbackAllowed = !isSplitTestEnabled('force-new-compile-domain') + + 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) 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 a084165473..5920d9e161 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 @@ -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 }) 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 7cd7269a94..4a2a40a571 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 @@ -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, }) diff --git a/services/web/frontend/js/features/user-content-domain-access-check/index.ts b/services/web/frontend/js/features/user-content-domain-access-check/index.ts index f817dc2924..3d0583ab93 100644 --- a/services/web/frontend/js/features/user-content-domain-access-check/index.ts +++ b/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) diff --git a/services/web/frontend/js/shared/context/detach-compile-context.js b/services/web/frontend/js/shared/context/detach-compile-context.js index 84f00cc889..e3320a37d5 100644 --- a/services/web/frontend/js/shared/context/detach-compile-context.js +++ b/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, diff --git a/services/web/frontend/js/shared/context/local-compile-context.js b/services/web/frontend/js/shared/context/local-compile-context.js index 352d2290c3..c557459624 100644 --- a/services/web/frontend/js/shared/context/local-compile-context.js +++ b/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, diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 9928210de2..11dd7cdd6a 100644 --- a/services/web/locales/en.json +++ b/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__. This could be caused by network blocking or a strict browser plugin rule. Please follow our <1>troubleshooting guide.", "new_file": "New File", "new_folder": "New Folder", "new_name": "New Name", diff --git a/services/web/test/unit/src/Compile/CompileControllerTests.js b/services/web/test/unit/src/Compile/CompileControllerTests.js index 9d1ca505d1..87166a236a 100644 --- a/services/web/test/unit/src/Compile/CompileControllerTests.js +++ b/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', }) ) })