Skip to content

fix(web): await or handle the floating promises the linter suppressed - #1219

Merged
mforce merged 2 commits into
mainfrom
fix/1167-floating-promises
Oct 11, 2026
Merged

mforce merged 2 commits into
mainfrom
fix/1167-floating-promises

Conversation

@mforce

@mforce mforce commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

Closes #1167

Removes all 16 @typescript-eslint/no-floating-promises suppressions left on main. Entries for other rules are untouched.

What changed

Tests (12 sites). farmCodeCache.test.ts now awaits rememberFarmCode. The quota test asserts .resolves and no longer uses not.toThrow(), because an async function never throws synchronously, so the old assertion could not fail. client.test.ts keeps each fake "other tab" lock hold and awaits it after releasing it, so a rejection in the hold now fails the test.

navigate() in Login.tsx (2) and AppLayout.tsx (1). The app uses <BrowserRouter>, where navigate() returns undefined. Only a data router returns a Promise. React Router types the return as void | Promise<void> and documents a module augmentation for declarative mode. App.tsx applies it beside <BrowserRouter>. These calls have no promise and no failure path, so there is nothing to test at runtime. The mutation proof is the linter: without the augmentation, lint reports the same three sites again.

Dashboard.tsx (1). The page-level verdict chain (Promise.allSettled(...).then(...)) had no rejection handler. It now logs with console.error("dashboard: panels verdict failed", err), the same pattern client.ts uses. allSettled never rejects and the callback only sets state, so no real input reaches this path today. The new test makes Promise.allSettled resolve with null once, so the verdict callback itself throws.

Proof

  • The new Dashboard test fails with Unhandled Rejection: TypeError when the .catch is removed, and also when it is moved before .then. With the code as shipped, it passes. (Codex review, round 1, caught that the first version of this test missed the second mutation.)
  • npm run lint -- --prune-suppressions leaves 0 no-floating-promises entries, and node lint/check-suppressions.mjs passes.
  • npm run lint and npm run typecheck are clean.
  • npx vitest run --maxWorkers=2 on client.test.ts, farmCodeCache.test.ts, Dashboard.test.tsx, Login.test.tsx and AppLayout.test.tsx: 384 passed.

Screenshots

None. Nothing a user sees changes. The navigate change is type-only, and the Dashboard handler writes to the console on a path no current input reaches.

Not changed

Login.tsx:93 keeps void previewConsent(...). That function catches every error and returns null, so it cannot reject.

Tests await the promises they start. Under BrowserRouter navigate() returns
nothing, so App.tsx applies React Router's documented declarative-mode type
augmentation. The Dashboard's page-level verdict chain now logs a failure
instead of leaving an unhandled rejection. Prunes every no-floating-promises
suppression.

Refs #1167
@mforce mforce added this to the Platform hardening milestone Oct 11, 2026
Codex review: rejecting Promise.allSettled itself skipped the verdict
callback, so a catch placed before .then still passed. A null result now
makes the callback throw.

Refs #1167
@mforce

mforce commented Oct 11, 2026

Copy link
Copy Markdown
Owner Author

Codex review, round 1 (local review, no inline thread). Finding: the Dashboard verdict test made Promise.allSettled itself reject, so the verdict callback never ran. Moving the .catch before .then still passed, while a throw inside the verdict went unhandled.

Fixed in 8f8e8ea. The test now resolves allSettled with null, so the verdict's own issued.filter throws a TypeError, and it asserts that console.error receives that TypeError.

Red/green, run on -t verdict:

  • Mutation A, allSettled(...).catch(log).then(verdict): RED, Unhandled Rejection: TypeError: Cannot read properties of null (reading 'filter').
  • Mutation B, no .catch at all: RED, same unhandled TypeError.
  • The PR's code restored: GREEN. All of Dashboard.test.tsx passes (135/135), and npm run lint and npm run typecheck are clean.

@mforce
mforce merged commit 113f6dc into main Oct 11, 2026
21 of 22 checks passed
@mforce
mforce deleted the fix/1167-floating-promises branch October 11, 2026 06: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.

Fix the 21 suppressed floating promises in the web app

1 participant