Merge pull request #33972 from overleaf/mj-web-show-pandoc-error

[web] Expose conversion errors during project exports

GitOrigin-RevId: 2e808bd65f03e81405db4727f2f5773d3b14cbe7

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

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,

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)

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'),

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": "",

services/web/frontend/js/features/ide-react/components/toolbar/export-document-toasts.tsx

@@ -6,8 +6,11 @@ const PreparingExportToast = () => {
   return <span>{t('preparing_for_export')}</span>
 }
 
-const ExportDocumentErrorToast = () => {
+const ExportDocumentErrorToast = ({ data }: { data?: any }) => {
   const { t } = useTranslation()
+  const errorMessage =
+    typeof data?.errorMessage === 'string' ? data.errorMessage : null
+
   return (
     <>
       <p>
@@ -20,6 +23,16 @@ const ExportDocumentErrorToast = () => {
           <a href="/contact" target="_BLANK" rel="noopener noreferrer" />,
         ]}
       />
+      {errorMessage && (
+        <details>
+          <summary>{t('error_details')}</summary>
+          <pre
+            style={{ maxWidth: '800px', maxHeight: '300px', overflow: 'auto' }}
+          >
+            <code>{errorMessage}</code>
+          </pre>
+        </details>
+      )}
     </>
   )
 }
@@ -70,10 +83,11 @@ const ExportDocumentSuccessToast = ({ data }: { data?: any }) => {
 const generators: GlobalToastGeneratorEntry[] = [
   {
     key: 'export-document:error',
-    generator: () => ({
-      content: <ExportDocumentErrorToast />,
+    generator: (data: any) => ({
+      content: <ExportDocumentErrorToast data={data} />,
       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 },
     })
   )
 }

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])

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.",

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 () {

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 () {

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 =