fix(media): guard against an upload result without Transloadit results - #2063
giladresisi wants to merge 2 commits into
Conversation
A second upload batch started while the first is in flight ends with no assembly; the complete handler crashed reading results (50 events/7d in prod). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017g3KqiuR5TT68XdqpTRbZh
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
| @@ -224,7 +224,13 @@ export function useUppyUploader(props: { | |||
|
|
|||
| if (transloadit.length > 0) { | |||
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Valid point, thanks. An empty results object was not one of the shapes behind the Sentry issue (those were a missing or empty transloadit array), but it would have fallen through to onUploadSuccess with an empty list and unlocked the editor silently. Extended the guard in 7c83f82 to treat an empty results object the same way: reset, warning toast, return. Retested the two batch race locally: no exception, toast shown.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017g3KqiuR5TT68XdqpTRbZh
What kind of change does this PR introduce?
Bug fix (frontend, media uploader). In the Uppy
completehandler inapps/frontend/src/components/media/new.uploader.tsx, the Transloadit branch now reads the assembly results with optional chaining (result.transloadit?.[0]?.results) and, when they are missing or an empty object, unlocks the editor, resets the file-order counter, shows the existing toaster with "Upload failed, please try again" and returns. The success path, the local-storage branch and the save-media calls are unchanged.Why was this change needed?
Sentry CLOUD-141 (
TypeError: Cannot read properties of undefined (reading 'results')and its sibling(reading '0'), same issue) has 50 events from 13 users in the last 7 days on/mediaand on the Instagram standalone connect page, last seen today.Root cause, reproduced locally against real Transloadit: the uploader runs with
autoProceed, so adding a second file while the first batch is still uploading starts a second batch.@uppy/transloaditkeeps a single current assembly and clears it when a batch finishes or errors, so the later batch reaches its post-processing step with no assembly and addstransloadit: []to the result. The handler then readresult.transloadit[0].resultsand threw. The(reading '0')variant is Uppy emittingcompletewith a bare{ successful, failed, uploadID }result (notransloaditkey) when the upload was removed mid-flight. The guard covers both shapes.Other information:
Two related things deliberately left out:
errorhandler in the same file callsuppy.clear()during an upload, which Uppy forbids and which is Sentry CLOUD-S1; separate PR.QA
TRANSLOADIT_*set, so the Media page uploads through Transloadit rather than local storage)/media, and add two small images one right after the other through the Upload input (fast enough that the second is added while the first is still uploading)TypeError: Cannot read properties of undefined (reading 'results')fromnew.uploader.tsxTested locally exactly as above: step 3 reproduced with the guard stashed (exact production message), step 4 verified with the guard in place across several runs (no exception, toast text present in the DOM); in some runs the first file was saved and in others neither file was, which is the race described above. The
(reading '0')variant was derived from@uppy/core's#runUploadresult fallback and not reproduced.Checklist:
🤖 Generated with Claude Code
https://claude.ai/code/session_017g3KqiuR5TT68XdqpTRbZh