Skip to content

Catch rejected onUpdate promises: revert + surface error instead of leaking unhandled rejection#314

Merged
CarlosNZ merged 5 commits into
v2.0-devfrom
copilot/bug-rejected-onupdate-promise
Jun 3, 2026
Merged

Catch rejected onUpdate promises: revert + surface error instead of leaking unhandled rejection#314
CarlosNZ merged 5 commits into
v2.0-devfrom
copilot/bug-rejected-onupdate-promise

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 3, 2026

A throwing async onUpdate escaped handleEdit as an unhandled rejection: the edit was silently lost, edit mode closed as if it succeeded, no error UI rendered, and on newer Node/Jest the rejection terminated the process.

const onUpdate = async () => { throw new Error('boom') }
<JsonEditor data={{ x: 'hello' }} setData={setData} onUpdate={onUpdate} />
// edit + Enter → setData never called, no error shown, unhandledRejection fires

Changes

  • src/JsonEditor.tsxhandleEdit: wrap await onUpdateRef.current(input) in try/catch. On rejection, surface err.message (or a bare-string throw) verbatim through the existing error slug, falling back to the event-specific default (ERROR_ADD / ERROR_DELETE / ERROR_UPDATE) — same fallback as the result === false branch.
  • No setData on rejection — deliberately matching the existing reject contract (see the comment above the false branch): a slow async rejection must not clobber a newer commit that landed while it was in flight. Display revert is already driven by the node consuming the returned error string, identical to the async-false path.
  • test/JsonEditor.test.tsx: promote the #271 test.todo to a real test — throws new Error('boom'), asserts setData is not called, original value still rendered, 'boom' visible in the DOM, and no unhandledRejection fires (regression guard against removing the try/catch).

Behaviour matrix (rejection only — other branches unchanged)

onUpdate outcome setData Error shown
throw new Error(m) no m
throw 'str' no 'str'
throw (no message) no event-specific default

Copilot AI changed the title [WIP] Fix silent loss of edits on update promise rejection Catch rejected onUpdate promises: revert + surface error instead of leaking unhandled rejection Jun 3, 2026
Copilot AI requested a review from CarlosNZ June 3, 2026 13:28
Base automatically changed from feat/v2.0-api-standardisation to v2.0-dev June 3, 2026 21:15
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

Bundle size impact

json-edit-react

Format Base raw PR raw Δ raw Base gzip PR gzip Δ gzip
esm 54.78 KB 54.89 KB 🔺 +119 B (+0.21%) 19.00 KB 19.03 KB 🔺 +32 B (+0.16%)
cjs 56.26 KB 56.37 KB 🔺 +119 B (+0.21%) 19.03 KB 19.05 KB 🔺 +28 B (+0.14%)

Measured from build/index.{cjs,esm}.js. Gzip at level 9.

Comment thread src/JsonEditor.tsx Outdated
Comment on lines +248 to +254
const errorKey =
input.event === 'add'
? 'ERROR_ADD'
: input.event === 'delete'
? 'ERROR_DELETE'
: 'ERROR_UPDATE'
return translateRef.current(errorKey, rootNodeDataRef.current)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the new ERROR_MESSAGE_KEY to correctly map error output strings

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — replaced the inline ternary with ERROR_MESSAGE_KEY[input.event] so the rejection fallback now covers rename/move too (ERROR_RENAME/ERROR_MOVE) instead of collapsing them to ERROR_UPDATE.

Copilot AI requested a review from CarlosNZ June 3, 2026 22:59
@CarlosNZ CarlosNZ marked this pull request as ready for review June 3, 2026 23:05
@CarlosNZ CarlosNZ merged commit b9ed44e into v2.0-dev Jun 3, 2026
2 checks passed
@CarlosNZ CarlosNZ deleted the copilot/bug-rejected-onupdate-promise branch June 3, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: rejected onUpdate Promise silently loses edits + leaks unhandled rejection

2 participants