[Obsidian review blockers] - Step 4: Contain async UI work - #2786
[Obsidian review blockers] - Step 4: Contain async UI work#2786logancyang wants to merge 1 commit into
Conversation
82b7e3c to
9e2d765
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e2d765ac7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| void navigator.clipboard | ||
| .readText() |
There was a problem hiding this comment.
Catch synchronous clipboard API failures
When this control runs in a WebView or environment where navigator.clipboard or readText is unavailable, evaluating this expression throws before a promise exists, so the chained .catch() never runs and clicking the paste button raises an uncaught exception. The previous try/catch also covered these synchronous failures; retain that boundary while explicitly discarding the returned promise.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid for this abandoned diff, but no longer applicable to the retained stack. PR #2786 was closed without merge; the current top stack at 3e257cb2 retains the async try/catch around navigator.clipboard.readText(), covering both synchronous API-access failures and rejected reads. I would not make a new change because the unsafe boundary change never shipped.
Conservative risk dispositionRisk: High — deferred and removed from the required stack. This PR changes async callback and lifecycle boundaries across core React UI. Those edits can affect event timing, unmount behavior, error propagation, duplicate work, or test scheduling even when the visible intent is only lint compliance. The current findings are warning-only. Recommendation: close this PR and revisit individual warnings only with focused behavioral tests; no async/UI lifecycle changes should ride with the Obsidian review blockers. |
Relates to logancyang/obsidian-copilot-preview#285
Why
Obsidian's review identifies floating promises and async callbacks passed to void-returning UI and plugin lifecycle boundaries. Rejections at those boundaries can otherwise escape without context, while changing every callback into ad hoc
voidexpressions would make the error policy inconsistent.What
This step adds one small
toVoidHandlerboundary helper that preserves callback arguments and reports synchronous throws and rejected promises through the existing logger. Affected React handlers, Obsidian modal callbacks, gallery lifecycle hooks, and plugin lifecycle entry points now make their fire-and-forget behavior explicit without changing the underlying async work or ordering.Non goal
Screenshot
Not applicable — UI structure, copy, styling, and interaction outcomes are unchanged.
Risk
High
Review: start with
src/utils/asyncHandler.tsandsrc/main.ts, then inspect representative call sites insrc/components/Chat.tsx,src/agentMode/ui/AgentChatInput.tsx, andsrc/settings/v2/components/PlusSettings.tsx.Verification
npm test -- --runInBand dev/gallery/main.test.ts src/utils/asyncHandler.test.ts.npm test -- --runInBand src/modelManagement/ui/dialogs/ConfigureProviderDialog.test.tsx src/agentMode/ui/AgentChatInput.test.tsx src/settings/v2/components/PlusSettings.test.tsx.npm run build.Note
npm run formatpasses. The staged-file lint gate passes with one pre-existing test-only mobile warning in the gallery fixture. Repo-widenpm run lintstill reports later review families tracked by issue #285; those are intentionally isolated into subsequent stack steps.