From 51ca5c015676b88241478c64ebcd8e3eb6e60f52 Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Thu, 28 May 2026 08:25:11 +0100 Subject: [PATCH] Merge pull request #33972 from overleaf/mj-web-show-pandoc-error [web] Expose conversion errors during project exports GitOrigin-RevId: 2e808bd65f03e81405db4727f2f5773d3b14cbe7 --- .../Downloads/ProjectDownloadsController.mjs | 6 ++ .../web/app/src/Features/Errors/Errors.js | 3 + .../Uploads/DocumentConversionManager.mjs | 38 +++++++-- .../Uploads/ProjectUploadController.mjs | 12 ++- .../web/frontend/extracted-translations.json | 1 + .../toolbar/export-document-toasts.tsx | 41 ++++++++-- .../ide-react/hooks/use-convert-project.ts | 14 +++- services/web/locales/en.json | 1 + .../ProjectDownloadsController.test.mjs | 45 +++++++++++ .../DocumentConversionManager.test.mjs | 80 ++++++++++++++++++- .../Uploads/ProjectUploadController.test.mjs | 57 ++++++++++++- 11 files changed, 281 insertions(+), 17 deletions(-) diff --git a/services/web/app/src/Features/Downloads/ProjectDownloadsController.mjs b/services/web/app/src/Features/Downloads/ProjectDownloadsController.mjs index b728a5a275..34b135f080 100644 --- a/services/web/app/src/Features/Downloads/ProjectDownloadsController.mjs +++ b/services/web/app/src/Features/Downloads/ProjectDownloadsController.mjs @@ -12,6 +12,7 @@ import Validation from '../../infrastructure/Validation.mjs' import { expressify } from '@overleaf/promise-utils' import { pipeline } from 'node:stream/promises' import SplitTestHandler from '../SplitTests/SplitTestHandler.mjs' +import { DocumentConversionError } from '../Errors/Errors.js' const { z, zz, parseReq } = Validation @@ -109,6 +110,11 @@ async function exportProjectConversion(req, res) { status: 'failure', operation: 'export', }) + if (error instanceof DocumentConversionError) { + return res.status(422).json({ + error: error.message, + }) + } throw error } const { conversionId, buildId, clsiServerId, file } = conversionResult diff --git a/services/web/app/src/Features/Errors/Errors.js b/services/web/app/src/Features/Errors/Errors.js index 7472af7008..313ab60bee 100644 --- a/services/web/app/src/Features/Errors/Errors.js +++ b/services/web/app/src/Features/Errors/Errors.js @@ -60,6 +60,8 @@ class UnsupportedFileTypeError extends BackwardCompatibleError {} class FileTooLargeError extends BackwardCompatibleError {} +class DocumentConversionError extends OError {} + class UnsupportedExportRecordsError extends BackwardCompatibleError {} class V1HistoryNotSyncedError extends BackwardCompatibleError {} @@ -399,6 +401,7 @@ module.exports = { InvalidNameError, UnsupportedFileTypeError, FileTooLargeError, + DocumentConversionError, UnsupportedExportRecordsError, V1HistoryNotSyncedError, ProjectHistoryDisabledError, diff --git a/services/web/app/src/Features/Uploads/DocumentConversionManager.mjs b/services/web/app/src/Features/Uploads/DocumentConversionManager.mjs index d4e818313c..e126568c21 100644 --- a/services/web/app/src/Features/Uploads/DocumentConversionManager.mjs +++ b/services/web/app/src/Features/Uploads/DocumentConversionManager.mjs @@ -14,7 +14,19 @@ import { import { pipeline } from 'node:stream/promises' import OError from '@overleaf/o-error' import FormData from 'form-data' -import { FileTooLargeError } from '../Errors/Errors.js' +import { FileTooLargeError, DocumentConversionError } from '../Errors/Errors.js' + +function extractClsiUserFacingError(error) { + try { + const parsed = JSON.parse(error.body) + if (typeof parsed?.error === 'string') { + return parsed.error + } + } catch { + // body wasn't JSON + } + return undefined +} async function convertDocumentToLaTeXZipArchive(path, userId, conversionType) { const clsiUrl = new URL(Settings.apis.clsi.url) @@ -76,6 +88,12 @@ async function convertDocumentToLaTeXZipArchive(path, userId, conversionType) { throw error } + if (error?.response?.status === 422) { + throw new DocumentConversionError( + extractClsiUserFacingError(error) + ).withCause(error) + } + throw new OError('document conversion failed').withCause(error) } @@ -148,10 +166,20 @@ async function convertProjectToDocumentOnce( 'sending project to CLSI for document conversion' ) - const { json, response } = await fetchJsonWithResponse(clsiUrl, { - method: 'POST', - json: clsiRequest, - }) + let json, response + try { + ;({ json, response } = await fetchJsonWithResponse(clsiUrl, { + method: 'POST', + json: clsiRequest, + })) + } catch (error) { + if (error?.response?.status === 422) { + throw new DocumentConversionError( + extractClsiUserFacingError(error) + ).withCause(error) + } + throw error + } const { conversionId, buildId, file } = json const clsiServerId = ClsiManager.CLSI_COOKIES_ENABLED ? ClsiManager.getClsiServerIdFromResponse(response) diff --git a/services/web/app/src/Features/Uploads/ProjectUploadController.mjs b/services/web/app/src/Features/Uploads/ProjectUploadController.mjs index bb0970c3b7..db3cb27ca9 100644 --- a/services/web/app/src/Features/Uploads/ProjectUploadController.mjs +++ b/services/web/app/src/Features/Uploads/ProjectUploadController.mjs @@ -13,7 +13,11 @@ import { InvalidZipFileError } from './ArchiveErrors.mjs' import multer from 'multer' import lodash from 'lodash' import { expressify } from '@overleaf/promise-utils' -import { DuplicateNameError, FileTooLargeError } from '../Errors/Errors.js' +import { + DuplicateNameError, + FileTooLargeError, + DocumentConversionError, +} from '../Errors/Errors.js' import DocumentConversionManager from './DocumentConversionManager.mjs' import ProjectOptionsHandler from '../Project/ProjectOptionsHandler.mjs' import AnalyticsManager from '../Analytics/AnalyticsManager.mjs' @@ -239,6 +243,12 @@ async function importDocument(req, res, next) { error: 'file_too_large', }) } + if (error instanceof DocumentConversionError) { + return res.status(422).json({ + success: false, + error: error.message || req.i18n.translate('upload_failed'), + }) + } res.status(500).json({ success: false, error: req.i18n.translate('upload_failed'), diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 108aa10242..91081dd7cd 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -624,6 +624,7 @@ "entry_plural": "", "equation_preview": "", "error": "", + "error_details": "", "error_loading_references": "", "error_opening_document": "", "error_opening_document_detail": "", diff --git a/services/web/frontend/js/features/ide-react/components/toolbar/export-document-toasts.tsx b/services/web/frontend/js/features/ide-react/components/toolbar/export-document-toasts.tsx index bf9c57b33e..4d8b89059a 100644 --- a/services/web/frontend/js/features/ide-react/components/toolbar/export-document-toasts.tsx +++ b/services/web/frontend/js/features/ide-react/components/toolbar/export-document-toasts.tsx @@ -6,8 +6,11 @@ const PreparingExportToast = () => { return {t('preparing_for_export')} } -const ExportDocumentErrorToast = () => { +const ExportDocumentErrorToast = ({ data }: { data?: any }) => { const { t } = useTranslation() + const errorMessage = + typeof data?.errorMessage === 'string' ? data.errorMessage : null + return ( <>

@@ -20,6 +23,16 @@ const ExportDocumentErrorToast = () => { , ]} /> + {errorMessage && ( +

+ {t('error_details')} + 
+            {errorMessage}
+
+
+ )} ) } @@ -70,10 +83,11 @@ const ExportDocumentSuccessToast = ({ data }: { data?: any }) => { const generators: GlobalToastGeneratorEntry[] = [ { key: 'export-document:error', - generator: () => ({ - content: , + generator: (data: any) => ({ + content: , type: 'error', - autoHide: true, + // Only auto-hide if we have no extra details + autoHide: !data?.errorMessage, delay: 5000, isDismissible: true, }), @@ -101,10 +115,25 @@ const generators: GlobalToastGeneratorEntry[] = [ export default generators -export const showExportDocumentError = () => { +// We only ever care about the latest error toast, so use a static handle. +const EXPORT_DOCUMENT_ERROR_HANDLE = 'export-document-error' + +export const showExportDocumentError = (errorMessage?: string) => { window.dispatchEvent( new CustomEvent('ide:show-toast', { - detail: { key: 'export-document:error' }, + detail: { + key: 'export-document:error', + handle: EXPORT_DOCUMENT_ERROR_HANDLE, + errorMessage, + }, + }) + ) +} + +export const hideExportDocumentError = () => { + window.dispatchEvent( + new CustomEvent('ide:dismiss-toast', { + detail: { handle: EXPORT_DOCUMENT_ERROR_HANDLE }, }) ) } diff --git a/services/web/frontend/js/features/ide-react/hooks/use-convert-project.ts b/services/web/frontend/js/features/ide-react/hooks/use-convert-project.ts index 2e170d971f..474e84f583 100644 --- a/services/web/frontend/js/features/ide-react/hooks/use-convert-project.ts +++ b/services/web/frontend/js/features/ide-react/hooks/use-convert-project.ts @@ -1,9 +1,10 @@ -import { getJSON } from '@/infrastructure/fetch-json' +import { FetchError, getJSON } from '@/infrastructure/fetch-json' import { useLocation } from '@/shared/hooks/use-location' import { debugConsole } from '@/utils/debugging' import { useProjectContext } from '@/shared/context/project-context' import { useCallback } from 'react' import { + hideExportDocumentError, hidePreparingExportToast, showExportDocumentError, showExportDocumentSuccess, @@ -35,6 +36,7 @@ export default function useConvertProject( url.searchParams.set('responseFormat', 'json') const { rootResourcePath } = getRootDocInfo() url.searchParams.set('rootResourcePath', rootResourcePath) + hideExportDocumentError() try { await openDocs.awaitBufferedOps(AbortSignal.timeout(10_000)) const response = await getJSON(url.href) @@ -49,7 +51,15 @@ export default function useConvertProject( } } catch (error) { hidePreparingToast() - showExportDocumentError() + let errorMessage + if ( + error instanceof FetchError && + error.response?.status === 422 && + error.data?.error + ) { + errorMessage = error.data.error + } + showExportDocumentError(errorMessage) debugConsole.error(error) } }, [projectId, type, getRootDocInfo, openDocs, location]) diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 329e1bbf77..f0d377d537 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -814,6 +814,7 @@ "equation_preview": "Equation preview", "error": "Error", "error_assist": "Error Assist", + "error_details": "Error details", "error_loading_references": "References couldn’t be loaded. Refresh the page to try again.", "error_opening_document": "Error opening document", "error_opening_document_detail": "Sorry, something went wrong opening this document. Please try again.", diff --git a/services/web/test/unit/src/Downloads/ProjectDownloadsController.test.mjs b/services/web/test/unit/src/Downloads/ProjectDownloadsController.test.mjs index bca3d11b9e..2965d5b24a 100644 --- a/services/web/test/unit/src/Downloads/ProjectDownloadsController.test.mjs +++ b/services/web/test/unit/src/Downloads/ProjectDownloadsController.test.mjs @@ -9,6 +9,11 @@ import { vi } from 'vitest' import sinon from 'sinon' import MockRequest from '../helpers/MockRequest.mjs' import MockResponse from '../helpers/MockResponse.mjs' +import { DocumentConversionError } from '../../../../app/src/Features/Errors/Errors.js' + +vi.mock('../../../../app/src/Features/Errors/Errors.js', () => + vi.importActual('../../../../app/src/Features/Errors/Errors.js') +) const modulePath = '../../../../app/src/Features/Downloads/ProjectDownloadsController.mjs' @@ -535,6 +540,46 @@ describe('ProjectDownloadsController', function () { sinon.assert.calledWith(ctx.pipeline, ctx.exportStream, ctx.res) }) }) + + describe('when conversion fails with a DocumentConversionError', function () { + beforeEach(async function (ctx) { + ctx.projectId = '5e9b1c2a3b4c5d6e7f8a9b0c' + ctx.userId = 'test-user-id' + ctx.req.params = { Project_id: ctx.projectId, type: 'docx' } + ctx.req.query = {} + ctx.req.session = { user: { _id: ctx.userId } } + + ctx.res.json = sinon.stub().returns(ctx.res) + + ctx.SessionManager.getLoggedInUserId.returns(ctx.userId) + ctx.DocumentConversionManager.promises.convertProjectToDocument.rejects( + new DocumentConversionError('parse error at line 5') + ) + + await ctx.ProjectDownloadsController.exportProjectConversion( + ctx.req, + ctx.res, + ctx.next + ) + }) + + it('should respond with 422 and the pandoc message', function (ctx) { + expect(ctx.res.statusCode).to.equal(422) + sinon.assert.calledWith(ctx.res.json, { + error: 'parse error at line 5', + }) + }) + + it('should not call next', function (ctx) { + sinon.assert.notCalled(ctx.next) + }) + + it('should not attempt to stream a document', function (ctx) { + sinon.assert.notCalled( + ctx.DocumentConversionManager.promises.streamConvertedProjectDocument + ) + }) + }) }) describe('downloadPreparedProjectExport', function () { diff --git a/services/web/test/unit/src/Uploads/DocumentConversionManager.test.mjs b/services/web/test/unit/src/Uploads/DocumentConversionManager.test.mjs index 42cf4677fc..bc8482cb6c 100644 --- a/services/web/test/unit/src/Uploads/DocumentConversionManager.test.mjs +++ b/services/web/test/unit/src/Uploads/DocumentConversionManager.test.mjs @@ -1,7 +1,10 @@ import { describe, expect, vi, beforeEach } from 'vitest' import sinon from 'sinon' import FormData from 'form-data' -import { FileTooLargeError } from '../../../../app/src/Features/Errors/Errors.js' +import { + FileTooLargeError, + DocumentConversionError, +} from '../../../../app/src/Features/Errors/Errors.js' const MODULE_PATH = '../../../../app/src/Features/Uploads/DocumentConversionManager.mjs' @@ -246,6 +249,30 @@ describe('DocumentConversionManager', function () { }) }) + describe('when CLSI returns a 422 with a user-facing JSON body', function () { + it('should reject with a DocumentConversionError carrying the pandoc message', async function (ctx) { + const clsiError = new Error('Bad Request') + clsiError.response = { status: 422 } + clsiError.body = JSON.stringify({ + error: 'parse error at line 5', + exitCode: 64, + }) + ctx.fetchUtils.fetchStreamWithResponse.rejects(clsiError) + + await expect( + ctx.DocumentConversionManager.promises.convertDocumentToLaTeXZipArchive( + '/path/to/input.docx', + 'test-user-id', + 'docx' + ) + ).to.be.rejectedWith( + sinon.match + .instanceOf(DocumentConversionError) + .and(sinon.match.has('message', 'parse error at line 5')) + ) + }) + }) + describe('when the converted archive is too large', function () { beforeEach(async function (ctx) { ctx.path = '/path/to/input.docx' @@ -326,7 +353,11 @@ describe('DocumentConversionManager', function () { await ctx.DocumentConversionManager.promises.convertProjectToDocument( ctx.projectId, ctx.userId, - ctx.type + ctx.type, + { + compileFromHistory: false, + rootDocPath: 'main.tex', + } ) }) @@ -371,6 +402,51 @@ describe('DocumentConversionManager', function () { }) }) }) + + describe('when CLSI returns a 422 with a user-facing JSON body', function () { + it('should reject with a DocumentConversionError carrying the pandoc message', async function (ctx) { + const clsiError = new Error('Bad Request') + clsiError.response = { status: 422 } + clsiError.body = JSON.stringify({ error: 'parse error at line 5' }) + ctx.fetchUtils.fetchJsonWithResponse.rejects(clsiError) + + await expect( + ctx.DocumentConversionManager.promises.convertProjectToDocument( + 'project-id', + 'user-id', + 'docx', + { + compileFromHistory: false, + rootDocPath: 'main.tex', + } + ) + ).to.be.rejectedWith( + sinon.match + .instanceOf(DocumentConversionError) + .and(sinon.match.has('message', 'parse error at line 5')) + ) + }) + }) + + describe('when CLSI returns a non-422 error', function () { + it('should rethrow the original error', async function (ctx) { + const clsiError = new Error('boom') + clsiError.response = { status: 500 } + ctx.fetchUtils.fetchJsonWithResponse.rejects(clsiError) + + await expect( + ctx.DocumentConversionManager.promises.convertProjectToDocument( + 'project-id', + 'user-id', + 'docx', + { + compileFromHistory: false, + rootDocPath: 'main.tex', + } + ) + ).to.be.rejectedWith('boom') + }) + }) }) describe('streamConvertedProjectDocument', function () { diff --git a/services/web/test/unit/src/Uploads/ProjectUploadController.test.mjs b/services/web/test/unit/src/Uploads/ProjectUploadController.test.mjs index ebe9895814..d8609dd1dd 100644 --- a/services/web/test/unit/src/Uploads/ProjectUploadController.test.mjs +++ b/services/web/test/unit/src/Uploads/ProjectUploadController.test.mjs @@ -10,7 +10,14 @@ import sinon from 'sinon' import MockRequest from '../helpers/MockRequest.mjs' import MockResponse from '../helpers/MockResponse.mjs' import ArchiveErrors from '../../../../app/src/Features/Uploads/ArchiveErrors.mjs' -import { FileTooLargeError } from '../../../../app/src/Features/Errors/Errors.js' +import { + FileTooLargeError, + DocumentConversionError, +} from '../../../../app/src/Features/Errors/Errors.js' + +vi.mock('../../../../app/src/Features/Errors/Errors.js', () => + vi.importActual('../../../../app/src/Features/Errors/Errors.js') +) const modulePath = '../../../../app/src/Features/Uploads/ProjectUploadController.mjs' @@ -693,6 +700,54 @@ describe('ProjectUploadController', function () { }) }) + describe('when the conversion fails with a user-facing error', async function () { + beforeEach(async function (ctx) { + ctx.DocumentConversionManager.promises.convertDocumentToLaTeXZipArchive = + sinon + .stub() + .rejects(new DocumentConversionError('parse error at line 5')) + + await new Promise(resolve => { + ctx.res.json = data => { + expect(data).to.deep.equal({ + success: false, + error: 'parse error at line 5', + }) + resolve() + } + ctx.ProjectUploadController.importDocument(ctx.req, ctx.res) + }) + }) + + it('should return http 422', function (ctx) { + expect(ctx.res.statusCode).to.equal(422) + }) + + it('should unlink the uploaded file', function (ctx) { + expect(ctx.fsPromises.unlink).to.have.been.calledWith(ctx.req.file.path) + }) + }) + + describe('when the conversion fails without a specific message', async function () { + beforeEach(async function (ctx) { + ctx.DocumentConversionManager.promises.convertDocumentToLaTeXZipArchive = + sinon.stub().rejects(new DocumentConversionError()) + + await new Promise(resolve => { + ctx.res.json = data => { + expect(data.success).to.equal(false) + expect(data.error).to.equal('upload_failed') + resolve() + } + ctx.ProjectUploadController.importDocument(ctx.req, ctx.res) + }) + }) + + it('should return http 422', function (ctx) { + expect(ctx.res.statusCode).to.equal(422) + }) + }) + describe('when the converted archive is too large', async function () { beforeEach(async function (ctx) { ctx.DocumentConversionManager.promises.convertDocumentToLaTeXZipArchive =