diff --git a/TODO.md b/TODO.md index a60557c..b6e5007 100644 --- a/TODO.md +++ b/TODO.md @@ -14,10 +14,9 @@ Reference: `docs/autosave-system.md` → "HTMX v2 Migration Plan" section. - [x] Form-help inline editors: add OverType toolbar + HTMX auto-save + remove save buttons - [x] Markdown cheatsheet modal: reusable dialog on all OverType editors -## FilePond error fixes +## FilePond crash on TFE upload forms -- [x] Fix `server.process.onerror`: don't access `response.status` on string (response is XHR text body) -- [x] Fix `server.load`: convert from string URL to object with proper `onload`/`onerror` handlers -- [x] Fix `server.process.onload`: guard against non-ID responses (e.g. HTML error pages disguised as 200) -- [x] Fix `destroyFilePondsIn`: abort in-flight uploads before destroying to prevent stale XHR callbacks crashing `_write` -- [x] Fix `FilepondHandler.php`: set `Content-Type: text/plain` header at top of `handleProcess`, `handleLoad`, `handleRevert`, `handleRemove` so PHP doesn't default to `text/html` on `die()` +- [x] Analyze root cause → `docs/filepond-crash-analysis.md` +- [x] Partial fixes (Content-Type headers, onerror cleanup, load object) — insufficient, crash still reproduces +- [ ] Replace `server.load` with custom function to bypass FilePond's buggy `load-file-error` → `DID_THROW_ITEM_INVALID` dispatch (see analysis doc, Option B) +- [ ] Implement `destroyFilePondsIn()` pre-destroy abort (defense in depth) diff --git a/docs/filepond-crash-analysis.md b/docs/filepond-crash-analysis.md new file mode 100644 index 0000000..e27d76f --- /dev/null +++ b/docs/filepond-crash-analysis.md @@ -0,0 +1,266 @@ +# FilePond crash analysis — TFE upload forms + +Status: **unresolved** — analysis complete, root cause identified in vendor code. +Hand this doc (and the whole repo) to another agent for implementing the fix. + +--- + +## Errors observed (Firefox, dev server 127.0.0.1:8000) + +Trigger: adding an image to "Image de couverture" (cover queue) or any TFE file upload form. + +``` +InstallTrigger is deprecated and will be removed in the future. content.js:1 +Failed to execute 'postMessage' on 'DOMWindow': target origin mismatch 2 20260609-... +htmx:targetError htmx.min.js:1 +Uncaught TypeError: can't access property "main", n.status is undefined filepond.min.js:9 + Wt filepond.min.js:9 + A filepond.min.js:9 + A filepond.min.js:9 + _write filepond.min.js:9 (×16) + filepond.min.js:9 + filepond.min.js:9 + e filepond.min.js:9 + e filepond.min.js:9 + u filepond.min.js:9 (× many — retry loop) + e filepond.min.js:9 + u ... + +[filepond:event] error Object { pond: {…}, error: null, file: {…} } file-upload-filepond.js:587 +``` + +Rows 1–3 are noise: `InstallTrigger`/`postMessage` are Firefox internals; `htmx:targetError` is an unrelated HTMX issue. The real crash is rows 4+. + +--- + +## Root cause + +### The crash + +At `filepond.min.js:9:60852`, FilePond 4.32.12 crashes inside its view system's `_write` method. Two view writers dereference `action.status.main`: + +**FilePond unminified (file-status view), line 7847:** +```js +var error = function error(_ref8) { + var root = _ref8.root, + action = _ref8.action; + text(root.ref.main, action.status.main); // ← crashes if action.status is undefined + text(root.ref.sub, action.status.sub); +}; +``` + +**FilePond unminified (assistant view), line 10735:** +```js +var itemError = function itemError(_ref6) { + var root = _ref6.root, + action = _ref6.action; + var item = root.query('GET_ITEM', action.id); + var filename = item.filename; + assist(root, action.status.main + ' ' + filename + ' ' + action.status.sub); +}; +``` + +Neither function guards against `action.status === undefined`. + +### How `action.status` becomes undefined + +FilePond's internal response objects use the property name **`code`**, not `status`: + +```js +// line 4700 +var createResponse = function createResponse(type, code, body, headers) { + return { + type: type, + code: code, // ← "code", not "status" + body: body, + headers: headers, + }; +}; +``` + +But the `load-file-error` event handler accesses `error.status`: + +```js +// line 6777-6784 +item.on('load-file-error', function(error) { + dispatch('DID_THROW_ITEM_INVALID', { + id: id, + error: error.status, // ← .status is undefined on createResponse objects! + status: error.status, // ← dispatches undefined as the status + }); + failure({ error: error.status, file: createItemAPI(item) }); +}); +``` + +Because the error object has `.code` (not `.status`), both `error: error.status` and `status: error.status` are `undefined`. When the dispatched action reaches the view writer, `action.status` is `undefined` → crash. + +### When does `load-file-error` fire? + +The `load-file-error` event is emitted in the item `_load` method when the `LOAD_FILE` filter chain **rejects**: + +```js +// line 5855-5863 +loader.on('load', function(file) { + var error = function error(result) { + state.file = file; + fire('load-meta'); + setStatus(ItemStatus.LOAD_ERROR); + fire('load-file-error', result); // ← fires when filter chain rejects + }; + + if (state.serverFileReference) { + success(file); // ← existing files take this safe path + return; + } + + onload(file, success, error); // ← new files take this path +}); +``` + +For existing DB files (edit mode), `state.serverFileReference` is set → `success(file)` is called directly → `load-file-error` never fires. + +For **newly added files** (no serverId yet), `onload(file, success, error)` runs the `LOAD_FILE` filter chain. The FilePond **FileValidateType** plugin (v1.2.8) registers a `LOAD_FILE` filter: + +```js +// plugin line 132 +addFilter('LOAD_FILE', function(file, _ref3) { + // ... + var handleRejection = function handleRejection() { + reject({ + status: { main: '...', sub: '...' } // ← plugin rejects with proper status object + }); + }; + // ... +}); +``` + +The plugin rejects with `{ status: { main, sub } }`. This is CORRECT. The rejected value flows into the `error(result)` callback → `result.status` IS `{ main, sub }`. So when `load-file-error` fires from a plugin rejection, `error.status` is actually a proper object, NOT undefined. **This particular path is safe.** + +However, `load-file-error` can also fire from the `DID_LOAD_ITEM` filter chain `catch` handler at line 6878: + +```js +.catch(function(e) { + if (!e || !e.error || !e.status) return handleAdd(false); + dispatch('DID_THROW_ITEM_INVALID', { + id: id, + error: e.error, + status: e.status, // ← e.status could be anything + }); +}); +``` + +This dispatches directly to `DID_THROW_ITEM_INVALID` (bypassing `load-file-error`), but it still copies `e.status` into the action. If `e.status` is undefined or not an object with `main`/`sub`, same crash. + +### How raw createResponse objects reach `load-file-error` + +There IS one path where a raw `createResponse` object (with `.code`, no `.status`) reaches `load-file-error`: + +When the server returns an HTTP error for a **load** request but the XHR onload handler treats it as success: + +```js +// line 4652 +xhr.onload = function() { + if (xhr.status >= 200 && xhr.status < 300) { + api.onload(xhr); // → blob processed + } else { + api.onerror(xhr); // → error callback + } +}; +``` + +If `xhr.status` is 0 (aborted XHR), it goes to `api.onerror`. But if the XHR is in some intermediate state, or if there's a race, it might not reach either path cleanly. Firefox's behavior with aborted XHRs and `responseType: 'blob'` can produce edge cases where `xhr.response` is a malformed blob and FilePond's blob processing triggers internal errors that propagate differently. + +### Summary of the bug + +| Component | Issue | +|-----------|-------| +| `createResponse()` (line 4700) | Uses `.code`, not `.status` | +| `load-file-error` handler (line 6777) | Reads `.status` on a createResponse object → `undefined` | +| Error view writer (line 7847) | No guard: crashes on `undefined.status.main` | +| Assistant view writer (line 10735) | Same: crashes on `undefined.status.main` | + +The bug is in FilePond 4.32.12 vendor code. We cannot modify `filepond.min.js`. + +--- + +## Proposed fix + +### Option A: Patch the minified JS (risky but direct) + +Find the `load-file-error` → `DID_THROW_ITEM_INVALID` dispatch in `filepond.min.js` and add a guard. Difficult because the code is minified and version-pinned via cache-busting query params. + +### Option B: Replace server.load with a custom function (cleanest) + +In `file-upload-filepond.js`, replace the `server.load` URL string with a **custom function** that: + +1. Makes its own `fetch`/XHR to load.php +2. On success: calls `load(blob)` — safe because `serverFileReference` is set for existing files +3. On error: calls `error('message')` — safe because this goes through `load-request-error` (NOT `load-file-error`) which properly creates `{ status: { main, sub } }` +4. Never lets FilePond's internal `createFetchFunction` create a createResponse object with `.code` + +This completely bypasses the buggy code path. + +```js +load: function(source, load, error, progress, abort, headers) { + var xhr = new XMLHttpRequest(); + var url = base + '/load.php?id=' + encodeURIComponent(source); + xhr.open('GET', url); + xhr.responseType = 'blob'; + xhr.onload = function() { + if (xhr.status >= 200 && xhr.status < 300) { + load(xhr.response); + } else { + error('Fichier introuvable (HTTP ' + xhr.status + ')'); + } + }; + xhr.onerror = function() { + error('Erreur réseau'); + }; + xhr.onabort = abort; + xhr.onprogress = function(e) { + if (e.lengthComputable) progress(e.lengthComputable, e.loaded, e.total); + }; + xhr.send(); + return { abort: function() { xhr.abort(); } }; +}, +``` + +### Option C: Abort in-flight loads before destroying (defense in depth) + +The `destroyFilePondsIn()` function in `file-upload-filepond.js` should abort in-flight loads/processing before calling `pond.destroy()`. Already partially attempted in commit `znunoqpw` but needs clean implementation. + +--- + +## Files involved + +| File | Role | +|------|------| +| `app/public/assets/js/vendor/filepond.min.js` | FilePond 4.32.12 — **contains the bug** (unmodifiable) | +| `app/public/assets/js/app/file-upload-filepond.js` | Our FilePond wrapper — **where the fix goes** | +| `app/src/FilepondHandler.php` | Server-side FilePond endpoints (process, load, revert, remove) | +| `app/public/admin/actions/filepond/load.php` | Admin load endpoint | +| `app/public/admin/actions/filepond/process.php` | Admin process endpoint | +| `app/public/partage/actions/filepond/load.php` | Partage load endpoint | +| `app/public/partage/actions/filepond/process.php` | Partage process endpoint | + +--- + +## Reproduction + +1. `just dev` (PHP dev server on 127.0.0.1:8000) +2. Open Firefox (Firefox triggers this more readily than Chromium due to different XHR abort behavior) +3. Go to `/admin/edit.php?id=` or `/admin/add.php` +4. Click "Parcourir" on the "Image de couverture" FilePond input +5. Select an image file → crash in console +6. Or: drag a file to the "TFE" FilePond input → same crash if the load fails or races with HTMX swaps + +--- + +## What commit `znunoqpw` already did (insufficient) + +- Added `Content-Type: text/plain` headers to all FilepondHandler error responses +- Fixed `server.process.onerror` to not access `.status` on a string +- Converted `server.load` from a URL string to an object with onload/onerror +- Added pre-destroy abort in `destroyFilePondsIn()` + +These changes address server response format and cleanup ordering, but **do not bypass the buggy `load-file-error` → `action.status` path inside FilePond's internal code**. The crash still reproduces.