Enable no-floating-promises and make fire-and-forget explicit#1855
Draft
kmcginnes wants to merge 8 commits into
Draft
Enable no-floating-promises and make fire-and-forget explicit#1855kmcginnes wants to merge 8 commits into
kmcginnes wants to merge 8 commits into
Conversation
Turn on the oxlint typescript/no-floating-promises rule and resolve every violation across the codebase. Persisted Jotai atom writes return a background persistence promise that was previously dropped silently, hiding real failures behind intentional fire-and-forget calls. Add a fireAndForget helper that observes failures via logger.error, and apply two idioms by meaning: fireAndForget for unawaited writes whose rejection should be logged (persisted-atom setters), and bare void for genuinely inert promises (navigation, self-catching helpers, test-state seeding). Record the convention in an ADR and the TypeScript conventions skill.
An explicitly-ignored rejection is non-blocking by construction, so warn better reflects its severity than error. Prefix the log so a reader knows the rejection came from a deliberately-unawaited promise.
Use `promise.catch(logAndIgnore)` instead of `fireAndForget(promise)`. The catch-handler form reads left-to-right and avoids wrapping each call in an extra paren and indent level, which is clearest for the multi-line setter blocks. Behavior is unchanged: rejections are still logged at warn level.
Where a test seeds a localForage-backed atom inside an async body or beforeEach, await the write instead of marking it void. This fails the test on a seeding error and removes the unhandled-rejection race at teardown. Seeds inside the synchronous renderHookWithJotai initializeState callback stay void, since its return is ignored and awaiting there is a no-op.
LoggerConnector.test.ts and localDb.test.ts call promise-returning writes inside async test bodies where the result can be awaited. Await them instead of marking void, so a failed write fails the test rather than surfacing as an unhandled rejection at teardown.
renderHookWithState, renderHookWithJotai, and DbState.applyTo now await the localForage-backed atom writes they seed, instead of leaving them as floating void promises. This means a seeding failure surfaces as a test failure and the unhandled-rejection race at teardown is gone. Both render helpers became async and renderHookWithJotai's initializeState now accepts an async callback, so all call sites await the helper (and their enclosing test functions are async). Sweeps the ~35 affected test files.
LoggerConnector methods now return Promise<void> instead of hiding the server-send promise behind a void contract. When sending a log to the server fails, ServerLoggerConnector logs a console warning and shows a warning toast, so the failure is no longer silent. Consumers that fire logging fire-and-forget (the explorers and schema fetchers) mark the now-visible promise with void; its rejection is handled inside the connector, so floating it is inert.
Add logAndNotify(message, options?), a .catch handler that logs at warn level and shows a warning toast, as the user-facing counterpart to logAndIgnore. Migrate every user-facing fire-and-forget persistence write to logAndNotify so a failed save is no longer silently swallowed: connections, schema cache, graph session, styles, namespace prefixes, settings, and layout preferences. Styling sites build the message with useTranslations so node/edge vocabulary reads correctly per query engine. The background canvas icon fetch keeps logAndIgnore since its failure is not worth interrupting the user.
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
1 participant
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.
Description
Turns on the oxlint
typescript/no-floating-promisesrule (previouslyoff) and resolves all 155 resulting violations across the codebase.Why: localForage-backed Jotai atom writes (
atomWithLocalForage) return a background persistence promise that callers dropped silently. A genuinely-swallowed persistence failure was indistinguishable from intentional fire-and-forget, hiding potential latent bugs. This rule makes ignoring a promise explicit everywhere.The convention (two idioms by meaning):
fireAndForget(promise)— new helper in@/utilsthat attaches.catch(logger.error). Used for unawaited writes whose rejection should be observed: persisted-atom setters. A failed durable write now gets logged instead of vanishing.void expr;— for genuinely inert promises: abortednavigate(), refetches whose errors surface via query state, self-catching async helpers, and test-state seeding.How to read:
packages/graph-explorer/src/utils/fireAndForget.ts(+ test) and.oxlintrc.json(rule flip). Start here.docs/adr/20260618-explicit-floating-promise-convention.md, with an agent-facing summary in.kiro/skills/typescript/SKILL.md.fireAndForget(...)orvoidper the convention. Production setters (~46 sites) and test seeding (~109 sites).This PR does not change the runtime behavior of persisted setters — it only makes the existing fire-and-forget explicit. Surfacing persistence failures to users (toast) is intentionally deferred.
Validation
pnpm checkspasses (lint, format, types).pnpm testpasses (162 files, 1811 passed, 1 expected-fail).fireAndForgethelper built test-first.Related Issues
Check List
pnpm checkspasses with no errors.pnpm testpasses with no failures.