fix(desktop): release profile backends before delete (#42613)
This commit is contained in:
@@ -4295,20 +4295,31 @@ async function teardownPrimaryBackendAndWait() {
|
|||||||
const dying = hermesProcess && !hermesProcess.killed ? hermesProcess : null
|
const dying = hermesProcess && !hermesProcess.killed ? hermesProcess : null
|
||||||
resetHermesConnection()
|
resetHermesConnection()
|
||||||
|
|
||||||
if (!dying) {
|
await waitForBackendExit(dying)
|
||||||
|
}
|
||||||
|
|
||||||
|
async function waitForBackendExit(child, timeoutMs = 5000) {
|
||||||
|
if (!child) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if (child.exitCode !== null || child.signalCode !== null) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
await new Promise(resolve => {
|
await new Promise(resolve => {
|
||||||
const timer = setTimeout(() => {
|
const timer = setTimeout(() => {
|
||||||
try {
|
try {
|
||||||
dying.kill('SIGKILL')
|
if (IS_WINDOWS && Number.isInteger(child.pid)) {
|
||||||
|
forceKillProcessTree(child.pid)
|
||||||
|
} else {
|
||||||
|
child.kill('SIGKILL')
|
||||||
|
}
|
||||||
} catch {
|
} catch {
|
||||||
// Already gone.
|
// Already gone.
|
||||||
}
|
}
|
||||||
resolve()
|
resolve()
|
||||||
}, 5000)
|
}, timeoutMs)
|
||||||
dying.once('exit', () => {
|
child.once('exit', () => {
|
||||||
clearTimeout(timer)
|
clearTimeout(timer)
|
||||||
resolve()
|
resolve()
|
||||||
})
|
})
|
||||||
@@ -4500,12 +4511,70 @@ function stopPoolBackend(profile) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function teardownPoolBackendAndWait(profile) {
|
||||||
|
const entry = backendPool.get(profile)
|
||||||
|
if (!entry) return
|
||||||
|
backendPool.delete(profile)
|
||||||
|
|
||||||
|
if (entry.process && !entry.process.killed) {
|
||||||
|
try {
|
||||||
|
entry.process.kill('SIGTERM')
|
||||||
|
} catch {
|
||||||
|
// Already gone.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
await waitForBackendExit(entry.process)
|
||||||
|
}
|
||||||
|
|
||||||
function stopAllPoolBackends() {
|
function stopAllPoolBackends() {
|
||||||
for (const profile of [...backendPool.keys()]) {
|
for (const profile of [...backendPool.keys()]) {
|
||||||
stopPoolBackend(profile)
|
stopPoolBackend(profile)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function profileNameFromDeleteRequest(request) {
|
||||||
|
if (!request || String(request.method || 'GET').toUpperCase() !== 'DELETE') {
|
||||||
|
return null
|
||||||
|
}
|
||||||
|
|
||||||
|
const match = String(request.path || '').match(/^\/api\/profiles\/([^/?#]+)(?:[?#].*)?$/)
|
||||||
|
if (!match) {
|
||||||
|
return null
|
||||||
|
}
|
||||||
|
|
||||||
|
let raw = ''
|
||||||
|
try {
|
||||||
|
raw = decodeURIComponent(match[1])
|
||||||
|
} catch {
|
||||||
|
return null
|
||||||
|
}
|
||||||
|
|
||||||
|
const name = raw.trim()
|
||||||
|
if (!name) {
|
||||||
|
return null
|
||||||
|
}
|
||||||
|
if (name.toLowerCase() === 'default') {
|
||||||
|
return 'default'
|
||||||
|
}
|
||||||
|
return name.toLowerCase()
|
||||||
|
}
|
||||||
|
|
||||||
|
async function prepareProfileDeleteRequest(request) {
|
||||||
|
const profile = profileNameFromDeleteRequest(request)
|
||||||
|
if (!profile || profile === 'default' || !PROFILE_NAME_RE.test(profile)) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if (profile === primaryProfileKey()) {
|
||||||
|
writeActiveDesktopProfile('default')
|
||||||
|
await teardownPrimaryBackendAndWait()
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
await teardownPoolBackendAndWait(profile)
|
||||||
|
}
|
||||||
|
|
||||||
async function startHermes() {
|
async function startHermes() {
|
||||||
// Latched-failure short-circuit: once bootstrap has failed in this
|
// Latched-failure short-circuit: once bootstrap has failed in this
|
||||||
// process, every subsequent startHermes() call re-throws the same error
|
// process, every subsequent startHermes() call re-throws the same error
|
||||||
@@ -5124,6 +5193,8 @@ ipcMain.handle('hermes:api', async (_event, request) => {
|
|||||||
return rerouted
|
return rerouted
|
||||||
}
|
}
|
||||||
|
|
||||||
|
await prepareProfileDeleteRequest(request)
|
||||||
|
|
||||||
const connection = await ensureBackend(request?.profile)
|
const connection = await ensureBackend(request?.profile)
|
||||||
const timeoutMs = resolveTimeoutMs(request?.timeoutMs, DEFAULT_FETCH_TIMEOUT_MS)
|
const timeoutMs = resolveTimeoutMs(request?.timeoutMs, DEFAULT_FETCH_TIMEOUT_MS)
|
||||||
const url = `${connection.baseUrl}${request.path}`
|
const url = `${connection.baseUrl}${request.path}`
|
||||||
|
|||||||
@@ -1010,6 +1010,7 @@ def delete_profile(name: str, yes: bool = False) -> Path:
|
|||||||
print(f"✓ Removed {wrapper_path}")
|
print(f"✓ Removed {wrapper_path}")
|
||||||
|
|
||||||
# 4. Remove profile directory
|
# 4. Remove profile directory
|
||||||
|
remove_error: Exception | None = None
|
||||||
try:
|
try:
|
||||||
def _make_writable(func, path, exc):
|
def _make_writable(func, path, exc):
|
||||||
"""onexc/onerror handler: add +w on PermissionError so rmtree can proceed.
|
"""onexc/onerror handler: add +w on PermissionError so rmtree can proceed.
|
||||||
@@ -1056,6 +1057,7 @@ def delete_profile(name: str, yes: bool = False) -> Path:
|
|||||||
print(f"✓ Removed {profile_dir}")
|
print(f"✓ Removed {profile_dir}")
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
print(f"⚠ Could not remove {profile_dir}: {e}")
|
print(f"⚠ Could not remove {profile_dir}: {e}")
|
||||||
|
remove_error = e
|
||||||
|
|
||||||
# 5. Clear active_profile if it pointed to this profile
|
# 5. Clear active_profile if it pointed to this profile
|
||||||
try:
|
try:
|
||||||
@@ -1066,6 +1068,9 @@ def delete_profile(name: str, yes: bool = False) -> Path:
|
|||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
if remove_error is not None:
|
||||||
|
raise RuntimeError(f"Could not remove profile directory {profile_dir}: {remove_error}") from remove_error
|
||||||
|
|
||||||
print(f"\nProfile '{canon}' deleted.")
|
print(f"\nProfile '{canon}' deleted.")
|
||||||
return profile_dir
|
return profile_dir
|
||||||
|
|
||||||
|
|||||||
@@ -442,6 +442,18 @@ class TestDeleteProfile:
|
|||||||
with pytest.raises(FileNotFoundError):
|
with pytest.raises(FileNotFoundError):
|
||||||
delete_profile("nonexistent", yes=True)
|
delete_profile("nonexistent", yes=True)
|
||||||
|
|
||||||
|
def test_rmtree_failure_raises(self, profile_env):
|
||||||
|
profile_dir = create_profile("coder", no_alias=True)
|
||||||
|
set_active_profile("coder")
|
||||||
|
|
||||||
|
with patch("hermes_cli.profiles._cleanup_gateway_service"), \
|
||||||
|
patch("hermes_cli.profiles.shutil.rmtree", side_effect=PermissionError("locked")):
|
||||||
|
with pytest.raises(RuntimeError, match="Could not remove profile directory"):
|
||||||
|
delete_profile("coder", yes=True)
|
||||||
|
|
||||||
|
assert profile_dir.is_dir()
|
||||||
|
assert get_active_profile() == "default"
|
||||||
|
|
||||||
|
|
||||||
# ===================================================================
|
# ===================================================================
|
||||||
# TestListProfiles
|
# TestListProfiles
|
||||||
|
|||||||
Reference in New Issue
Block a user