fix(import): surface real error cause in /pages/import instead of generic 400
The two catch blocks in importPage() threw an opaque "Error processing file
content" / "Failed to create imported page" BadRequest, hiding the real cause
from the HTTP response. This made a production 400 regression impossible to
diagnose without server log access, and violated the project convention that
errors must never be swallowed.
Extract `${err.name}: ${err.message}` into both the log (full err object kept
for the stack) and the thrown BadRequestException. Inner processMarkdown/
processHTML rethrowing catches and the EE processDocx/processPdf license
catches are left unchanged.
Local reproduction of the happy-dom 14->20 theory failed (full import chain
+ 22 edge cases pass on happy-dom@20.8.9), so the root cause is still pending
the now-visible reason from a recurring 400. Diagnostic script test-import.tsx
added; backlog doc updated with findings.
This commit is contained in:
@@ -91,9 +91,14 @@ export class ImportService {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
const message = 'Error processing file content';
|
// Surface the real cause instead of a generic mask, so the failure is
|
||||||
this.logger.error(message, err);
|
// diagnosable from the HTTP response (project convention: never swallow).
|
||||||
throw new BadRequestException(message);
|
const reason =
|
||||||
|
err instanceof Error ? `${err.name}: ${err.message}` : String(err);
|
||||||
|
this.logger.error(`Error processing file content: ${reason}`, err);
|
||||||
|
throw new BadRequestException(
|
||||||
|
`Error processing file content: ${reason}`,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!prosemirrorState) {
|
if (!prosemirrorState) {
|
||||||
@@ -129,9 +134,12 @@ export class ImportService {
|
|||||||
`Successfully imported "${title}${fileExtension}. ID: ${createdPage.id} - SlugId: ${createdPage.slugId}"`,
|
`Successfully imported "${title}${fileExtension}. ID: ${createdPage.id} - SlugId: ${createdPage.slugId}"`,
|
||||||
);
|
);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
const message = 'Failed to create imported page';
|
const reason =
|
||||||
this.logger.error(message, err);
|
err instanceof Error ? `${err.name}: ${err.message}` : String(err);
|
||||||
throw new BadRequestException(message);
|
this.logger.error(`Failed to create imported page: ${reason}`, err);
|
||||||
|
throw new BadRequestException(
|
||||||
|
`Failed to create imported page: ${reason}`,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
121
docs/backlog/pages-import-broken-400.md
Normal file
121
docs/backlog/pages-import-broken-400.md
Normal file
@@ -0,0 +1,121 @@
|
|||||||
|
# /pages/import отдаёт 400 «Error processing file content» (регресс)
|
||||||
|
|
||||||
|
Статус: **диагностируемость починена** (fix #1 применён); корневая причина **не
|
||||||
|
подтверждена** — на текущем коде локально баг воспроизвести не удалось.
|
||||||
|
Ниже — что удалось выяснить, главный подозреваемый и что проверить дальше.
|
||||||
|
|
||||||
|
## Симптом
|
||||||
|
|
||||||
|
На задеплоенном инстансе эндпоинт `POST /pages/import` отдаёт
|
||||||
|
`400 BadRequest` с телом «Error processing file content». Раньше работал —
|
||||||
|
похоже на регресс после редеплоя гитмоста.
|
||||||
|
|
||||||
|
Через этот эндпоинт грузит контент MCP-инструмент `create_page` (это
|
||||||
|
единственный эндпоинт, принимающий контент при создании страницы —
|
||||||
|
см. комментарий в `packages/mcp/src/client.ts:961`).
|
||||||
|
|
||||||
|
Что при этом **исправно** (важно для локализации):
|
||||||
|
- `POST /pages/create` — создание пустой страницы.
|
||||||
|
- `update_page_json` — запись контента через realtime-коллаборацию (Yjs).
|
||||||
|
|
||||||
|
## Где именно бросается ошибка
|
||||||
|
|
||||||
|
`apps/server/src/integrations/import/services/import.service.ts:93-97` —
|
||||||
|
`try/catch` вокруг обработки контента:
|
||||||
|
|
||||||
|
```ts
|
||||||
|
} catch (err) {
|
||||||
|
const message = 'Error processing file content';
|
||||||
|
this.logger.error(message, err); // реальная причина логируется ТОЛЬКО в логи
|
||||||
|
throw new BadRequestException(message); // наружу уходит generic-строка
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Реальный текст ошибки/стек **проглатывается** (наружу — generic-строка), что
|
||||||
|
нарушает конвенцию проекта (см. CLAUDE.md, «Errors must never be swallowed»).
|
||||||
|
Поэтому по ответу 400 причину не видно — её надо читать в логах сервера
|
||||||
|
(`logger.error(message, err)` пишет полный err) ИЛИ воспроизвести локально.
|
||||||
|
|
||||||
|
## Цепочка обработки для .md (что внутри try)
|
||||||
|
|
||||||
|
`importPage` → `processMarkdown(fileContent)`:
|
||||||
|
1. `markdownToHtml` (`packages/editor-ext/.../marked.utils.ts`) — marked, чистый JS, без DOM.
|
||||||
|
2. `processHTML`: cheerio `load` → `normalizeImportHtml` (`utils/import-formatter.ts`) — чистый JS.
|
||||||
|
3. `htmlToJson` (`apps/server/src/collaboration/collaboration.util.ts:118`) →
|
||||||
|
`generateJSON(html, tiptapExtensions)`.
|
||||||
|
|
||||||
|
## Ключевая зацепка: путь импорта зависит от happy-dom, рабочие пути — нет
|
||||||
|
|
||||||
|
`generateJSON` (`apps/server/src/common/helpers/prosemirror/html/generateJSON.ts`)
|
||||||
|
парсит HTML через **happy-dom**: `new Window()` + `new localWindow.DOMParser()` +
|
||||||
|
`parseFromString(...)`, затем `PMDOMParser.fromSchema(schema).parse(doc.body)`.
|
||||||
|
|
||||||
|
А исправные пути DOM-парсер НЕ используют:
|
||||||
|
- `/pages/create` — пустая страница, контент не парсится.
|
||||||
|
- `update_page_json` — пишет готовый ProseMirror-JSON в Yjs
|
||||||
|
(`TiptapTransformer.toYdoc`), без HTML→DOM.
|
||||||
|
|
||||||
|
То есть единственное, что есть в сломанном пути и отсутствует в рабочих, —
|
||||||
|
**серверный парсинг HTML через happy-dom**.
|
||||||
|
|
||||||
|
## Главный подозреваемый: бамп happy-dom (14 → 20)
|
||||||
|
|
||||||
|
- Изначально было `"happy-dom": "^14.12.3"`.
|
||||||
|
- Сейчас запинено `"happy-dom": "20.8.9"` в `apps/server/package.json:83`
|
||||||
|
(+ override в корневом `package.json`).
|
||||||
|
- Пин на `20.8.9` пришёл в коммите `17da7629 "overrides"`
|
||||||
|
(Philipinho, 2026-03-28), где `20.8.4` → `20.8.9`.
|
||||||
|
- Скачок 14 → 20 — это 6 мажоров; у happy-dom между мажорами ломающие
|
||||||
|
изменения в API `Window`/`DOMParser` и в поведении парсинга HTML. Очень
|
||||||
|
вероятно, что `generateJSON` ломается на новом happy-dom.
|
||||||
|
|
||||||
|
Версия в node_modules подтверждена: `happy-dom@20.8.9` (симлинк свежий).
|
||||||
|
|
||||||
|
## Второстепенный подозреваемый
|
||||||
|
|
||||||
|
`getSchema(tiptapExtensions)` / `PMDOMParser.parse(...)` могут спотыкаться на
|
||||||
|
`parseHTML`-правилах недавно добавленных нод (synced blocks/transclusion,
|
||||||
|
page break, indent, columns, status — все они в `tiptapExtensions`). Но
|
||||||
|
`getSchema` используется и в рабочем пути (`createYdoc`/`update_page_json`),
|
||||||
|
поэтому сам по себе билд схемы скорее всего цел — под подозрением именно
|
||||||
|
DOM-парс-ветка, уникальная для импорта.
|
||||||
|
|
||||||
|
## Направления фикса
|
||||||
|
|
||||||
|
1. **Диагностируемость — ✅ СДЕЛАНО (по конвенции проекта).** В catch-блоках
|
||||||
|
`import.service.ts` (обработка контента + вставка страницы) реальная
|
||||||
|
причина теперь прокидывается наружу: `BadRequestException` несёт
|
||||||
|
`${err.name}: ${err.message}`, а в лог пишется полный `err` со стеком.
|
||||||
|
Раньше наружу уходила generic-строка "Error processing file content".
|
||||||
|
Теперь при повторе 400 на проде реальный reason будет виден прямо в теле
|
||||||
|
ответа — без необходимости лезть в логи.
|
||||||
|
2. **Корневой фикс — ⏳ НЕ ПОДТВЕРЖДЁН.** Гипотеза happy-dom 14→20 **не
|
||||||
|
подтвердилась** при локальном воспроизведении на текущем коде (см. ниже).
|
||||||
|
Применять блайнд-даунгрейд happy-dom нельзя — нужен реальный stack из
|
||||||
|
логов/ответа после повторения.
|
||||||
|
|
||||||
|
## Локальное воспроизведение (выполнено)
|
||||||
|
|
||||||
|
На текущем `main` (happy-dom 20.8.9) вся цепочка импорта `.md` отработала
|
||||||
|
без ошибок через `tsx` (импорты прямо из source, не из dist):
|
||||||
|
|
||||||
|
- `markdownToHtml` → cheerio `load` → `normalizeImportHtml` → `generateJSON`
|
||||||
|
с полным набором из 44 `tiptapExtensions` — **OK** для:
|
||||||
|
- базового markdown (заголовки, bold/italic, списки, таблицы, code-block,
|
||||||
|
blockquote)
|
||||||
|
- edge-cases: пустой контент, whitespace, HTML-сущности, вложенные списки,
|
||||||
|
task-list, emoji, кириллица, спецсимволы в code, ссылки, изображения, hr
|
||||||
|
- API happy-dom 20.8.9, используемые в `generateJSON`, существуют и работают:
|
||||||
|
`new Window()`, `new localWindow.DOMParser()`, `parseFromString('…',
|
||||||
|
'text/html')`, `happyDOM.abort()` (async), `happyDOM.close()` (async).
|
||||||
|
- Блок `finally` в `generateJSON` вызывает `abort()/close()` без `await` и без
|
||||||
|
`try/catch`, но эти методы не бросают синхронно и не перезаписывают
|
||||||
|
результат — **не является** причиной 400 (проверено отдельным тестом).
|
||||||
|
- Все `parseHTML`-правила расширений (status, transclusion, page-break,
|
||||||
|
columns, subpages и т.д.) участвуют в успешном тесте — ни одно не падает.
|
||||||
|
|
||||||
|
Вывод: на текущем коде баг **не воспроизводится**. Вероятные объяснения —
|
||||||
|
контент-специфичный кейс, которого нет в тестах; разница между source и
|
||||||
|
собранным `dist`; либо временное состояние задеплоенного инстанса. После
|
||||||
|
применения fix #1 повторный 400 покажет реальный reason — по нему и искать
|
||||||
|
корень.
|
||||||
111
test-import.tsx
Normal file
111
test-import.tsx
Normal file
@@ -0,0 +1,111 @@
|
|||||||
|
// Diagnostic / reproduction script for the /pages/import 400 regression
|
||||||
|
// (see docs/backlog/pages-import-broken-400.md).
|
||||||
|
//
|
||||||
|
// Run from repo root: npx tsx test-import.tsx
|
||||||
|
//
|
||||||
|
// Exercises the full server-side import chain directly against source:
|
||||||
|
// markdownToHtml (@docmost/editor-ext)
|
||||||
|
// -> cheerio load + normalizeImportHtml
|
||||||
|
// -> generateJSON (happy-dom DOMParser -> ProseMirror) with all 44 tiptapExtensions
|
||||||
|
//
|
||||||
|
// Also pokes the happy-dom cleanup behavior used in generateJSON's `finally`
|
||||||
|
// block, to rule out the "finally throw masks the real result" footgun.
|
||||||
|
//
|
||||||
|
// If this script throws on some input, that input reproduces the prod 400 and
|
||||||
|
// the thrown error is the real cause hidden behind "Error processing file content".
|
||||||
|
|
||||||
|
import { markdownToHtml } from '@docmost/editor-ext';
|
||||||
|
import { generateJSON } from './apps/server/src/common/helpers/prosemirror/html/generateJSON';
|
||||||
|
import { tiptapExtensions } from './apps/server/src/collaboration/collaboration.util';
|
||||||
|
import { load } from 'cheerio';
|
||||||
|
import { normalizeImportHtml } from './apps/server/src/integrations/import/utils/import-formatter';
|
||||||
|
import { Window } from 'happy-dom';
|
||||||
|
|
||||||
|
// Mirror the exact server chain for a .md file.
|
||||||
|
async function processMd(md: string): Promise<any> {
|
||||||
|
const html = await markdownToHtml(md);
|
||||||
|
const $ = load(html);
|
||||||
|
normalizeImportHtml($, $.root());
|
||||||
|
const normalizedHtml = $.html() || '';
|
||||||
|
return generateJSON(normalizedHtml, tiptapExtensions);
|
||||||
|
}
|
||||||
|
|
||||||
|
let failures = 0;
|
||||||
|
function check(name: string, fn: () => void | Promise<void>) {
|
||||||
|
return Promise.resolve()
|
||||||
|
.then(() => fn())
|
||||||
|
.then(() => console.log(`✅ ${name}`))
|
||||||
|
.catch((err: any) => {
|
||||||
|
failures++;
|
||||||
|
console.error(`❌ ${name}: ${err?.name}: ${err?.message}`);
|
||||||
|
if (err?.stack) console.error(err.stack);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
async function main() {
|
||||||
|
console.log('=== Section A: full import chain (markdown -> PM JSON) ===');
|
||||||
|
const mdCases: Array<[string, string]> = [
|
||||||
|
['basic markdown', '# Title\n\n**bold** and *italic*.\n\n- a\n- b\n'],
|
||||||
|
['empty', ''],
|
||||||
|
['whitespace only', ' \n\n '],
|
||||||
|
['just title', '# Title'],
|
||||||
|
['html entities', '# Test & <code> "quotes" </code>'],
|
||||||
|
['nested lists', '# T\n\n- a\n - b\n - c\n- d'],
|
||||||
|
['task list', '- [ ] todo\n- [x] done'],
|
||||||
|
['emoji', '# Test 🎉 emoji ✓'],
|
||||||
|
['cyrillic', '# Заголовок\n\nТекст на русском'],
|
||||||
|
['code with special chars', '```\nconst x = "<>&"\n```'],
|
||||||
|
['link', '[example](https://example.com)'],
|
||||||
|
['image', ''],
|
||||||
|
['table', '| Col1 | Col2 |\n|------|------|\n| v1 | v2 |\n'],
|
||||||
|
['blockquote', '> quote\n> line2'],
|
||||||
|
];
|
||||||
|
for (const [name, md] of mdCases) {
|
||||||
|
await check(`md: ${name}`, () => processMd(md));
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log('\n=== Section B: raw generateJSON on tricky HTML fragments ===');
|
||||||
|
const htmlCases: Array<[string, string]> = [
|
||||||
|
['plain paragraph', '<p>Hello</p>'],
|
||||||
|
['deeply nested divs', '<div><div><div><p>deep</p></div></div></div>'],
|
||||||
|
['unclosed-ish tags (browser-fixup)', '<b>bold<i>both</b>italic'],
|
||||||
|
['empty body', ''],
|
||||||
|
['only whitespace nodes', ' \n '],
|
||||||
|
];
|
||||||
|
for (const [name, html] of htmlCases) {
|
||||||
|
await check(`html: ${name}`, () => generateJSON(html, tiptapExtensions));
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log('\n=== Section C: happy-dom cleanup behavior (generateJSON finally block) ===');
|
||||||
|
await check('sync finally with abort()/close() returns SUCCESS', () => {
|
||||||
|
// Mirrors generateJSON.ts finally exactly: no await, no try/catch.
|
||||||
|
const w = new Window();
|
||||||
|
try {
|
||||||
|
const dp = new w.DOMParser();
|
||||||
|
dp.parseFromString('<!DOCTYPE html><html><body><p>hi</p></body></html>', 'text/html');
|
||||||
|
} finally {
|
||||||
|
w.happyDOM.abort();
|
||||||
|
w.happyDOM.close();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
await check('abort()/close() are Promises (async) in happy-dom 20', async () => {
|
||||||
|
const w = new Window();
|
||||||
|
const a = w.happyDOM.abort();
|
||||||
|
const c = w.happyDOM.close();
|
||||||
|
if (!(a instanceof Promise) || !(c instanceof Promise)) {
|
||||||
|
throw new Error('expected abort/close to return Promises');
|
||||||
|
}
|
||||||
|
await a;
|
||||||
|
await c;
|
||||||
|
});
|
||||||
|
await check('double close() does not throw', () => {
|
||||||
|
const w = new Window();
|
||||||
|
w.happyDOM.close();
|
||||||
|
w.happyDOM.close();
|
||||||
|
});
|
||||||
|
|
||||||
|
console.log(`\n=== Done. Failures: ${failures} ===`);
|
||||||
|
if (failures > 0) process.exitCode = 1;
|
||||||
|
}
|
||||||
|
|
||||||
|
main();
|
||||||
Reference in New Issue
Block a user