Repository navigation
fix(fields): report a failed ImageField upload instead of leaking the rejection - #10310
Merged
os-project-manager merged 4 commits intoSep 24, 2026
Merged
Conversation
… rejection Both upload paths (picker and crop confirm) were try/finally with no catch, so a rejected upload rendered nothing and escaped as an unhandled promise rejection. Catch per pick / per crop and render the translated fields.file.uploadFailed message FileField shows for the same failure; the failed image is not added, successful picks of a multi-select still land. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D486C1axmXnrkJMNUfz2eb
The fields test program carries no Node types by design; reach process through globalThis with the two members the test uses. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D486C1axmXnrkJMNUfz2eb
…dule helper A component-scoped useCallback for the message made the React Compiler skip one more memoization (react-hooks/preserve-manual-memoization, 1 -> 2 on this file); a pure module-level function taking t, as maxSizeError does, adds none. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D486C1axmXnrkJMNUfz2eb
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contributor
Author
Contract reviewServed-tier: 62/62 Adopted from an isolated at-tier reviewer by ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-project-manager
marked this pull request as ready for review
September 24, 2026 18:31
os-project-manager
marked this pull request as draft
September 24, 2026 18:32
os-project-manager
deleted the
claude/issue-10226-image-upload-failure-reported
branch
September 24, 2026 19:11
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #10226
Clause-②: no
What changed
ImageField(@object-ui/fields) has two upload paths: the native picker (handleFileChange) and the crop dialog's confirm (handleCropConfirm). Onmainat8b1f066both ran the upload insidetry … finallywith nocatch. Both handlers are fire-and-forget: React ignores the promise anonChangehandler returns, andImageCropperDialogcallsonConfirm(blob, name)without awaiting it. So a rejected upload (network error, 413, storage outage) showed nothing and escaped as an unhandled promise rejection.This PR catches the failure on both paths and renders the same message
FileFieldrenders for the same failure through the sameuseUpload()transport: keyfields.file.uploadFailed, samename/errorarguments, samedefaultValue. The message goes in the widget's existing error row (the one the oversize-pick rejections already use, objectui#4141).FileField'suseFileUploads. A failed pick is reported and not added. In a multi-select, the picks that did upload still land, and the oversize rejections are kept beside the upload failures.onChange). The dialog still closes infinally, so the message shows in the field's error row instead of behind the dialog.uploadFailedMessage(t, name, err), in the same shape asmaxSizeError. A component-scopeduseCallbackmade the React Compiler skip one more memoization, so the helper lives outside the component.The key exists in every locale pack
@object-ui/i18nships.all-locales-key-parity.test.tsre-derives that, andpnpm check:i18n-keyschecks the call site's arguments and inlinedefaultValueagainst theenpack.Files:
packages/fields/src/widgets/ImageField.tsx,packages/fields/src/widgets/ImageField.uploadFailure.test.tsx(new),.changeset/10226-image-upload-failure-reported.md(patch,@object-ui/fields).Evidence (head
737663e)Red first, on unmodified source. I ran the new test against
8b1f066before touching the widget. All three failure cases went red, and each saw exactly oneunhandledRejectioncarryingnetwork down, with no failure text in the DOM:The two success controls (picker and crop, resolving transport) pass on both trees.
Ablation on the committed fix (head
737663e). A trap-guarded script replacedImageField.tsxwith its8b1f066blob, ran the test, and restored the file withgit checkout HEAD -- PATH.uploadFailedoccurrences went 4 → 0 andcatch (err)went to 0.Tests 3 failed | 2 passed (5), the same three failures as above.git diff HEADis empty.Local gates, on head
737663e, exit codes written to files before being read:pnpm --filter @object-ui/fields type-check(tsc --noEmit && tsc -p tsconfig.test.json) → exit 0. I built the dependency closure first withpnpm --filter '@object-ui/fields^...' build.pnpm exec vitest run packages/fields/→ exit 0,Test Files 179 passed | 1 skipped (180),Tests 3010 passed | 7 skipped (3017).eslinton the two touched.ts(x)files → 0 errors.ImageField.tsxhas 7 warnings against 6 on8b1f066; the one new warning is below.lint.ymlsets no--max-warnings.check:i18n-keys,check:i18n-dead-keys,check:vi-mock-specifiers,check:vi-mock-inherit,check:vi-mock-override-shape,check:new-line-citations,check:control-bytes,check:test-path-roots,check:unreferenced-sources,scripts/check-changeset-presence.mjs,scripts/check-changeset-no-major.mjs→ all exit 0.Scope of local runs: repo-wide
pnpm lint, the fullpnpm test, and every othercheck:*family were left to CI. I ran lint only on the touched files. That narrowing is not a proof that no other file's verdict can move, and I did not check whether type-aware rules are on ineslint.config.js.Acceptance notes
react-hooks/preserve-manual-memoizationnow also fires onhandleCropConfirm. Adding thecatchmakes the React Compiler skip memoizing that callback; it already skippedopenCropperonmain. This only affects optimization, and the file already carries the same warning.lint.ymlsets no--max-warnings.Errorrejections readundefined. For a rejection that is not anError,(err as Error).messageputsundefinedin the message.FileFielddoes exactly the same, and I copied its behaviour on purpose so the two widgets say the same thing. Carrier: none.FileFieldandImageFieldnow each build thefields.file.uploadFailedcall the same way.file-size-guard.tsexplains why the size guard is shared rather than copied per widget, and the same argument could apply to this message. Moving it would touchFileField.tsx, which is outside this claim's file surface. Carrier: none.ImageCropperDialog's title and description do not go throught(). I left them alone: they are not part of this defect and not in scope.2091538) has aCo-Authored-Bytrailer that names a model version. The later commits use the plainClaudeform. History was not rewritten.Session:
https://claude.ai/code/session_01D486C1axmXnrkJMNUfz2ebGenerated by Claude Code