fix(desktop): don't announce a reconnect when the app is quitting
Quitting the Desktop app wrote "[boot] Restarting desktop connection" to
desktop.log and pushed the same message to the renderer over
hermes:boot-progress. Nothing was restarting: the quit coordinator called
teardownPrimaryBackendAndWait() with the default soft=false, and soft=false
is what makes resetHermesConnectionState() call
resetBootProgressForReconnect().
Name the two teardown intents in quit-teardown.ts and use them at every
deliberate primary teardown: a teardown that re-homes ('reconnect', update
hand-off and bundle swap) keeps the announcement; one that brings nothing
back ('quit', the quit coordinator and the uninstall teardown) stays silent.
Fixes #123437.
This commit is contained in:
@@ -443,7 +443,7 @@ import {
|
||||
import { createQuickEntryShortcut, quickEntryWindowBounds, sanitizeQuickEntrySettings } from './quick-entry'
|
||||
import { createQuitFinalization } from './quit-finalization'
|
||||
import { type ActiveWork, backendOwnedByApp, mergeActiveWork, normalizeActiveWork, quitPromptFor } from './quit-guard'
|
||||
import { backendQuitNeedsWait, createQuitTeardownCoordinator, type QuitTeardownTask } from './quit-teardown'
|
||||
import { backendQuitNeedsWait, backendTeardownOptions, createQuitTeardownCoordinator, type QuitTeardownTask } from './quit-teardown'
|
||||
import * as remoteLifecycle from './remote-lifecycle'
|
||||
import {
|
||||
attachPowerResumeRemoteRevalidation,
|
||||
@@ -3637,7 +3637,10 @@ function isLightVariant(): boolean {
|
||||
/** Invalidate connections and wait for every owned backend before the swap. */
|
||||
async function teardownBundledBackend(): Promise<void> {
|
||||
isQuittingForHandoff = true
|
||||
const results = await Promise.allSettled([teardownPrimaryBackendAndWait(), stopAllPoolBackends()])
|
||||
const results = await Promise.allSettled([
|
||||
teardownPrimaryBackendAndWait(backendTeardownOptions('reconnect')),
|
||||
stopAllPoolBackends()
|
||||
])
|
||||
const errors = results.filter(result => result.status === 'rejected').map(result => result.reason)
|
||||
|
||||
if (errors.length) {
|
||||
@@ -4282,7 +4285,7 @@ function reapOrphanedBackendsOnce() {
|
||||
// `hermes update`; neither venv scans nor a second fleet stop belong here.
|
||||
async function stopBackendsForUpdate(): Promise<void> {
|
||||
if (IS_WINDOWS) {
|
||||
await Promise.all([teardownPrimaryBackendAndWait(), stopAllPoolBackends()])
|
||||
await Promise.all([teardownPrimaryBackendAndWait(backendTeardownOptions('reconnect')), stopAllPoolBackends()])
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4313,7 +4316,8 @@ async function releaseBackendLock(updateRoot: string, tag: string): Promise<{ un
|
||||
}
|
||||
}
|
||||
|
||||
await Promise.all([teardownPrimaryBackendAndWait(), stopAllPoolBackends()])
|
||||
// No backend comes back after an uninstall: stay silent, like a quit.
|
||||
await Promise.all([teardownPrimaryBackendAndWait(backendTeardownOptions('quit')), stopAllPoolBackends()])
|
||||
|
||||
// Uninstall deletes the whole runtime. Drain separately-running gateways
|
||||
// through the CLI, rather than targeting a gateway worker by PID.
|
||||
@@ -12475,7 +12479,7 @@ const backendShutdown = createBackendShutdownCoordinator(async (): Promise<void>
|
||||
const ownedChildren = IS_WINDOWS ? collectOwnedBackendChildren() : []
|
||||
const localShutdown = localBackendLifecycle.shutdown()
|
||||
const primary = backendConnectionState.getProcess()
|
||||
const primaryStop = teardownPrimaryBackendAndWait()
|
||||
const primaryStop = teardownPrimaryBackendAndWait(backendTeardownOptions('quit'))
|
||||
const pooledStops = stopAllPoolBackends()
|
||||
|
||||
if (poolIdleReaper) {
|
||||
|
||||
@@ -2,7 +2,7 @@ import assert from 'node:assert/strict'
|
||||
|
||||
import { test, vi } from 'vitest'
|
||||
|
||||
import { backendQuitNeedsWait, createQuitTeardownCoordinator } from './quit-teardown'
|
||||
import { backendQuitNeedsWait, backendTeardownOptions, createQuitTeardownCoordinator } from './quit-teardown'
|
||||
|
||||
function deferred() {
|
||||
let resolve!: () => void
|
||||
@@ -105,6 +105,15 @@ test('teardown failure still releases the final quit after all branches settle',
|
||||
assert.equal(requestFinalQuit.mock.calls.length, 1)
|
||||
})
|
||||
|
||||
test('only a teardown that brings a backend back may announce it', () => {
|
||||
// `soft` is what keeps a teardown out of resetBootProgressForReconnect(): the
|
||||
// "[boot] Restarting desktop connection" line in desktop.log and the
|
||||
// hermes:boot-progress push to the renderer. A shutdown must stay silent,
|
||||
// a teardown that re-homes may announce itself.
|
||||
assert.equal(backendTeardownOptions('quit').soft, true, 'a quit must not announce a reconnect')
|
||||
assert.equal(backendTeardownOptions('reconnect').soft, false, 'a re-home keeps the announcement')
|
||||
})
|
||||
|
||||
test('a synchronous reentrant quit cannot start a second teardown', async () => {
|
||||
const finalQuit = vi.fn()
|
||||
const coordinator = createQuitTeardownCoordinator(finalQuit)
|
||||
|
||||
@@ -25,6 +25,30 @@ export function backendQuitNeedsWait(activity: BackendQuitActivity): boolean {
|
||||
return activity.shutdownPending || activity.processAttached || activity.connectionPending || activity.poolPending
|
||||
}
|
||||
|
||||
/**
|
||||
* What a deliberate primary-backend teardown intends to happen next.
|
||||
*
|
||||
* `'reconnect'`: a backend comes back (update hand-off, bundle swap, re-home).
|
||||
* `'quit'`: nothing comes back — the app is exiting, or being uninstalled.
|
||||
*/
|
||||
export type BackendTeardownIntent = 'quit' | 'reconnect'
|
||||
|
||||
/**
|
||||
* The `soft` option a deliberate teardown must pass to
|
||||
* `teardownPrimaryBackendAndWait()`.
|
||||
*
|
||||
* `soft: true` is what stops `resetHermesConnectionState()` from rewriting the
|
||||
* boot-progress overlay — the step that writes `[boot] Restarting desktop
|
||||
* connection` into desktop.log and pushes `hermes:boot-progress` to the
|
||||
* renderer. A reconnect may announce that; a quit must not, because the
|
||||
* announcement is false, it can overwrite the renderer's own "Update in
|
||||
* progress…" copy, and a reader of desktop.log then attributes a shutdown to a
|
||||
* re-home.
|
||||
*/
|
||||
export function backendTeardownOptions(intent: BackendTeardownIntent): { soft: boolean } {
|
||||
return { soft: intent === 'quit' }
|
||||
}
|
||||
|
||||
function runTask(task: QuitTeardownTask): Promise<unknown> {
|
||||
try {
|
||||
return Promise.resolve(task.run())
|
||||
|
||||
Reference in New Issue
Block a user