Repository navigation
fix(runtime): wait on close for widget dev work already started, with a bound - #646
Merged
Merged
Conversation
… a bound `WidgetDevSessions.close()` now returns a promise that waits for every session's queued work, including a superseded snapshot still being removed with retries, for at most two seconds. The node's shutdown starts it with the other stops and awaits it before the database closes. The spec awaits `close()` and removes its folder with the async cleanup helper. The held-snapshot test no longer races a 300 ms timer against the retry budget: one attempt meets the hold, the holder exits, and then the runtime's own removal runs, whose retry options are asserted. Refs #626, #631 Fixes #639
Contributor
Author
|
Review attestation: ready to merge at A push to this PR makes this attestation stale; the new head needs its own review. |
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
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 #639. Refs #626, #631, #637.
What changed
WidgetDevSessions.close()now returns a promise. It stops watching every folder, then waits for each session's queued work, including a prune still removing a superseded snapshot with retries. The wait is bounded bycloseWaitMs, which defaults toWIDGET_DEV_CLOSE_WAIT_MS(2 s). That covers one snapshot's full retry budget of about 1.5 s and fits inside the node's 5 s shutdown grace. Past the bound it logs that it is closing anyway and resolves.SNAPSHOT_REMOVALis the promiserm, with 5 retries and a 100 ms delay.RuntimeHandlesgainscloseWidgetDev(), andstopUpdateChecks()no longer closes widget dev sessions.main.tsstartscloseWidgetDev()alongside the other stops and awaits it before the database closes. The only other callers are tests:slash-commands.spec.tsnow awaits it.widget-dev-sessions.spec.ts:afterEachawaitsclose(), thenawait removeTestDirectory(dir)instead ofrmSync.EBUSYorEPERMand is asserted on Windows. The holder is then released and has exited before the runtime's own removal runs. The test asserts that the runtime asked forSNAPSHOT_REMOVAL, with retries. Its outcome no longer depends on load.waits on close for a superseded snapshot still being removedandstops waiting on close after its bound, .... Thermmock can now hold a removal open on a gate.Verification
origin/main'swidget-dev-sessions.ts:expected true to be false(close resolved during the prune) andexpected 0 to be greater than or equal to 150. They pass on this branch.widget-dev-sessions.spec.tsandslash-commands.spec.ts, 72 tests passed.widget-dev-sessions.spec.ts, 20 rounds. 160 passed, 0 failed. Some rounds overlapped a fullpnpm verify.pnpm typecheck,pnpm invariants(15/15, includingtest-cleanup-retries-asynchronously), eslint on the changed files, andpnpm verify(575 files passed) all pass.No platform-specific behaviour was added. The hold assertion applies only on win32, where the hold has an effect.
Overlap: #644 also edits
apps/runtime/test/widget-dev-sessions.spec.ts, in the folder-recreate test around line 1080. This PR's spec edits are the mock andafterEachnear the top, and the held-snapshot test plus the new tests that follow it. No hunks are shared.