Skip to content

Commit 89fc26d

Browse files
Vrtak-CZclaude
andcommitted
fix(updater): propagate errors from apply update instead of silently failing
The "Restart to update" button had no visible effect when apply() failed because errors were swallowed at every layer. Now apply() throws on precondition failures, emits error status on extraction/swap failures, the RPC handler returns { ok: false, error } instead of always { ok: true }, and the frontend shows error text with a loading state during the attempt. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6bcd10c commit 89fc26d

7 files changed

Lines changed: 99 additions & 15 deletions

File tree

src/bun/index.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,12 @@ const rpc = BrowserView.defineRPC<KloviRPC>({
114114
},
115115
applyUpdate: async () => {
116116
const mgr = getUpdateManager();
117-
await mgr.apply();
118-
return { ok: true };
117+
try {
118+
await mgr.apply();
119+
return { ok: true };
120+
} catch (error) {
121+
return { ok: false, error: error instanceof Error ? error.message : "Update failed" };
122+
}
119123
},
120124
openExternal: (params) => {
121125
Utils.openExternal(params.url);

src/bun/updater.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ export class UpdateManager {
355355

356356
async apply(): Promise<void> {
357357
if (!this.downloadedAssetPath || !this.latestRelease) {
358-
return;
358+
throw new Error("No downloaded update to apply");
359359
}
360360

361361
const stagingDir = join(this.updatesDir(), "staging");
@@ -388,6 +388,12 @@ export class UpdateManager {
388388
try {
389389
rmSync(stagingDir, { recursive: true });
390390
} catch {}
391+
this.emitStatus({
392+
status: "error",
393+
currentVersion: this.currentVersion,
394+
latestVersion: this.latestRelease.tag_name,
395+
error: error instanceof Error ? error.message : "Update failed",
396+
});
391397
throw error;
392398
}
393399
}

src/frontend/components/UpdateNotification.css

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,15 @@
2222
cursor: pointer;
2323
}
2424

25-
.update-notification-action:hover {
25+
.update-notification-action:hover:not(:disabled) {
2626
opacity: 0.9;
2727
}
2828

29+
.update-notification-action:disabled {
30+
opacity: 0.6;
31+
cursor: default;
32+
}
33+
2934
.update-notification-dismiss {
3035
background: none;
3136
border: none;

src/frontend/components/UpdateNotification.test.tsx

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import { afterEach, describe, expect, mock, test } from "bun:test";
2-
import { cleanup, fireEvent, render } from "@testing-library/react";
2+
import { cleanup, fireEvent, render, waitFor } from "@testing-library/react";
33
import type { UpdateStatus } from "../../shared/rpc-types.ts";
44
import { setupMockRPC } from "../test-helpers/mock-rpc.ts";
55
import { UpdateNotification } from "./UpdateNotification.tsx";
66

77
const VERSION_READY_PATTERN = /v2\.0\.0 is ready/;
8+
const EXTRACT_FAILED_PATTERN = /Extract failed/;
9+
const UPDATE_FAILED_PATTERN = /Update failed/;
10+
const NETWORK_TIMEOUT_PATTERN = /Network timeout/;
811

912
function defaultProps() {
1013
return {
@@ -68,6 +71,44 @@ describe("UpdateNotification", () => {
6871
expect(applyUpdate).toHaveBeenCalled();
6972
});
7073

74+
test("shows Restarting text and disables button while applying", () => {
75+
const applyUpdate = mock(() => new Promise<{ ok: boolean }>(() => {})); // never resolves
76+
setupMockRPC({ applyUpdate });
77+
const props = defaultProps();
78+
props.status = { status: "ready", currentVersion: "1.0.0", latestVersion: "2.0.0" };
79+
const { getByRole } = render(<UpdateNotification {...props} />);
80+
fireEvent.click(getByRole("button", { name: "Restart to update" }));
81+
const button = getByRole("button", { name: "Restarting…" });
82+
expect(button).toBeDefined();
83+
expect(button.hasAttribute("disabled")).toBe(true);
84+
});
85+
86+
test("shows error when applyUpdate returns ok: false", async () => {
87+
const applyUpdate = mock(() => Promise.resolve({ ok: false, error: "Extract failed" }));
88+
setupMockRPC({ applyUpdate });
89+
const props = defaultProps();
90+
props.status = { status: "ready", currentVersion: "1.0.0", latestVersion: "2.0.0" };
91+
const { getByRole, getByText } = render(<UpdateNotification {...props} />);
92+
fireEvent.click(getByRole("button", { name: "Restart to update" }));
93+
await waitFor(() => {
94+
expect(getByText(EXTRACT_FAILED_PATTERN)).toBeDefined();
95+
});
96+
// Button should be re-enabled after error
97+
expect(getByRole("button", { name: "Restart to update" }).hasAttribute("disabled")).toBe(false);
98+
});
99+
100+
test("shows error when applyUpdate rejects", async () => {
101+
const applyUpdate = mock(() => Promise.reject(new Error("RPC error")));
102+
setupMockRPC({ applyUpdate });
103+
const props = defaultProps();
104+
props.status = { status: "ready", currentVersion: "1.0.0", latestVersion: "2.0.0" };
105+
const { getByRole, getByText } = render(<UpdateNotification {...props} />);
106+
fireEvent.click(getByRole("button", { name: "Restart to update" }));
107+
await waitFor(() => {
108+
expect(getByText(UPDATE_FAILED_PATTERN)).toBeDefined();
109+
});
110+
});
111+
71112
test("shows up-to-date message for manual check result", () => {
72113
const props = defaultProps();
73114
props.manualCheckResult = { status: "up-to-date", currentVersion: "1.0.0" };
@@ -83,7 +124,7 @@ describe("UpdateNotification", () => {
83124
error: "Network timeout",
84125
};
85126
const { getByText } = render(<UpdateNotification {...props} />);
86-
expect(getByText(/Network timeout/)).toBeDefined();
127+
expect(getByText(NETWORK_TIMEOUT_PATTERN)).toBeDefined();
87128
});
88129

89130
test("calls onDismissManualCheck when dismissing manual check result", () => {

src/frontend/components/UpdateNotification.tsx

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useState } from "react";
12
import type { UpdateStatus } from "../../shared/rpc-types.ts";
23
import { getRPC } from "../rpc.ts";
34
import "./UpdateNotification.css";
@@ -59,19 +60,46 @@ export function UpdateNotification({
5960
return null;
6061
}
6162

63+
return <ReadyBanner latestVersion={status.latestVersion} onDismiss={onDismiss} />;
64+
}
65+
66+
function ReadyBanner({
67+
latestVersion,
68+
onDismiss,
69+
}: {
70+
latestVersion: string;
71+
onDismiss: () => void;
72+
}) {
73+
const [applying, setApplying] = useState(false);
74+
const [error, setError] = useState<string | null>(null);
75+
76+
const handleApply = async () => {
77+
setApplying(true);
78+
setError(null);
79+
try {
80+
const result = await getRPC().request.applyUpdate({} as Record<string, never>);
81+
if (!result.ok) {
82+
setError(result.error ?? "Update failed");
83+
setApplying(false);
84+
}
85+
} catch {
86+
setError("Update failed");
87+
setApplying(false);
88+
}
89+
};
90+
6291
return (
6392
<div className="update-notification">
64-
<span className="update-notification-text">Klovi v{status.latestVersion} is ready</span>
93+
<span className="update-notification-text">
94+
{error ? `Update failed: ${error}` : `Klovi v${latestVersion} is ready`}
95+
</span>
6596
<button
6697
type="button"
6798
className="update-notification-action"
68-
onClick={() => {
69-
getRPC()
70-
.request.applyUpdate({} as Record<string, never>)
71-
.catch(() => {});
72-
}}
99+
disabled={applying}
100+
onClick={handleApply}
73101
>
74-
Restart to update
102+
{applying ? "Restarting…" : "Restart to update"}
75103
</button>
76104
<button
77105
type="button"

src/frontend/rpc.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export interface RPCClient {
4747
autoDownload?: boolean;
4848
}) => Promise<UpdateSettingsInfo>;
4949
checkForUpdate: (params: Record<string, never>) => Promise<UpdateStatus>;
50-
applyUpdate: (params: Record<string, never>) => Promise<{ ok: boolean }>;
50+
applyUpdate: (params: Record<string, never>) => Promise<{ ok: boolean; error?: string }>;
5151
openExternal: (params: { url: string }) => Promise<{ ok: boolean }>;
5252
browseDirectory: (params: { startingFolder?: string }) => Promise<{ path: string | null }>;
5353
};

src/shared/rpc-types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export interface KloviRPC {
9292
};
9393
applyUpdate: {
9494
params: Record<string, never>;
95-
response: { ok: boolean };
95+
response: { ok: boolean; error?: string };
9696
};
9797
openExternal: { params: { url: string }; response: { ok: boolean } };
9898
browseDirectory: {

0 commit comments

Comments
 (0)