From f51ae82d146a3e0cd76d39384fef3213751426d5 Mon Sep 17 00:00:00 2001 From: Tim deBoer Date: Mon, 10 Feb 2025 14:44:30 -0500 Subject: [PATCH 1/3] chore: using simple objects in rpc api (to avoid snapshots) Moving to svelte 5, state objects are proxied. Normally that's an implementation detail, but when you try to pass the proxies through rpc they can't be cloned and result in errors. We could use $state.snapshot(build) every time we pass objects from the UI to the API, but we really shouldn't be passing whole objects to the backend and blindly trusting that the objects (including all properties) match what the backend knows anyway. This changes the three affected API functions to accept ids instead of objects, and the backend looks up the objects before acting. Although it's slightly more work on the backend, this is a better/more robust API, and also avoids the svelte 5 proxy issue. Fixes #1304. Signed-off-by: Tim deBoer --- packages/backend/src/api-impl.ts | 27 ++++++++++++++----- .../lib/disk-image/DiskImageActions.spec.ts | 1 + .../lib/disk-image/DiskImageActions.svelte | 2 +- .../DiskImageDetailsVirtualMachine.spec.ts | 2 ++ .../DiskImageDetailsVirtualMachine.svelte | 4 +-- .../src/lib/disk-image/DiskImagesList.spec.ts | 1 + .../src/lib/disk-image/DiskImagesList.svelte | 2 +- packages/shared/src/BootcAPI.ts | 6 ++--- 8 files changed, 32 insertions(+), 13 deletions(-) diff --git a/packages/backend/src/api-impl.ts b/packages/backend/src/api-impl.ts index 3fc3654b..86aebd42 100644 --- a/packages/backend/src/api-impl.ts +++ b/packages/backend/src/api-impl.ts @@ -54,7 +54,11 @@ export class BootcApiImpl implements BootcApi { return checkPrereqs(await getContainerEngine()); } - async checkVMLaunchPrereqs(build: BootcBuildInfo): Promise { + async checkVMLaunchPrereqs(buildId: string): Promise { + const build = this.history.getHistory().find(build => build.id === buildId); + if (!build) { + return Promise.reject(new Error(`Could not find build: ${buildId}`)); + } return createVMManager(build).checkVMLaunchPrereqs(); } @@ -66,11 +70,16 @@ export class BootcApiImpl implements BootcApi { return buildDiskImage(build, this.history, overwrite); } - async launchVM(build: BootcBuildInfo): Promise { + async launchVM(buildId: string): Promise { try { - await createVMManager(build).launchVM(); - // Notify it has successfully launched - await this.notify(Messages.MSG_VM_LAUNCH_ERROR, { success: 'Launched!', error: '' }); + const build = this.history.getHistory().find(build => build.id === buildId); + if (!build) { + await this.notify(Messages.MSG_VM_LAUNCH_ERROR, { success: '', error: 'Could not find build to launch' }); + } else { + await createVMManager(build).launchVM(); + // Notify it has successfully launched + await this.notify(Messages.MSG_VM_LAUNCH_ERROR, { success: 'Launched!', error: '' }); + } } catch (e) { // Make sure that we are able to display the "stderr" information if it exists as that actually shows // the error when running the command. @@ -90,13 +99,18 @@ export class BootcApiImpl implements BootcApi { return stopCurrentVM(); } - async deleteBuilds(builds: BootcBuildInfo[]): Promise { + async deleteBuilds(buildIds: string[]): Promise { const response = await podmanDesktopApi.window.showWarningMessage( `Are you sure you want to remove the selected disk images from the build history? This will remove the history of the build as well as remove any lingering build containers.`, 'Yes', 'No', ); if (response === 'Yes') { + // create an array of builds. invalid build ids are ignored + const builds = buildIds + .map(id => this.history.getHistory().find(build => build.id === id)) + .filter(build => build) as BootcBuildInfo[]; + // Map each build to a delete operation promise const deletePromises = builds.map(build => this.deleteBuildContainer(build)); @@ -330,6 +344,7 @@ export class BootcApiImpl implements BootcApi { // this method is internal and meant to be used by the API implementation protected async notify(id: string, body: unknown = {}): Promise { // Must pass in an empty body, if it is undefined this fails + console.log('notify frontend: ' + id); await this.webview.postMessage({ id, body, diff --git a/packages/frontend/src/lib/disk-image/DiskImageActions.spec.ts b/packages/frontend/src/lib/disk-image/DiskImageActions.spec.ts index d08e10c0..d17886e0 100644 --- a/packages/frontend/src/lib/disk-image/DiskImageActions.spec.ts +++ b/packages/frontend/src/lib/disk-image/DiskImageActions.spec.ts @@ -73,6 +73,7 @@ test('Test clicking on delete button', async () => { deleteButton.click(); expect(spyOnDelete).toHaveBeenCalled(); + expect(spyOnDelete).toHaveBeenCalledWith(['name1']); }); test('Test clicking on logs button', async () => { diff --git a/packages/frontend/src/lib/disk-image/DiskImageActions.svelte b/packages/frontend/src/lib/disk-image/DiskImageActions.svelte index c08b53e2..2867eec0 100644 --- a/packages/frontend/src/lib/disk-image/DiskImageActions.svelte +++ b/packages/frontend/src/lib/disk-image/DiskImageActions.svelte @@ -16,7 +16,7 @@ let isWindows = $state(false); // Delete the build async function deleteBuild(): Promise { - await bootcClient.deleteBuilds([object]); + await bootcClient.deleteBuilds([object.id]); } // Navigate to the build diff --git a/packages/frontend/src/lib/disk-image/DiskImageDetailsVirtualMachine.spec.ts b/packages/frontend/src/lib/disk-image/DiskImageDetailsVirtualMachine.spec.ts index 40497ec8..cd8e3bcf 100644 --- a/packages/frontend/src/lib/disk-image/DiskImageDetailsVirtualMachine.spec.ts +++ b/packages/frontend/src/lib/disk-image/DiskImageDetailsVirtualMachine.spec.ts @@ -69,6 +69,8 @@ test('Render virtual machine terminal window', async () => { await waitFor(() => { expect(bootcClient.launchVM).toHaveBeenCalled(); }); + expect(bootcClient.checkVMLaunchPrereqs).toHaveBeenCalledWith('id1'); + expect(bootcClient.launchVM).toHaveBeenCalledWith('id1'); }); test('Show prereqs message if prereq check fails (returns ANY string)', async () => { diff --git a/packages/frontend/src/lib/disk-image/DiskImageDetailsVirtualMachine.svelte b/packages/frontend/src/lib/disk-image/DiskImageDetailsVirtualMachine.svelte index 366f1cd4..6f181483 100644 --- a/packages/frontend/src/lib/disk-image/DiskImageDetailsVirtualMachine.svelte +++ b/packages/frontend/src/lib/disk-image/DiskImageDetailsVirtualMachine.svelte @@ -185,7 +185,7 @@ async function launchVM(build: BootcBuildInfo): Promise { // This is launched IN THE BACKGROUND. We do not wait for the VM to boot before showing the terminal. // we instead are notified by subscribing to Messages.MSG_VM_LAUNCH_ERROR messages from RPC - bootcClient.launchVM(build).catch((e: unknown) => console.error('error launching VM', e)); + bootcClient.launchVM(build.id).catch((e: unknown) => console.error('error launching VM', e)); // Initialize the terminal so it awaits the websocket connection. await initTerminal(); @@ -261,7 +261,7 @@ onMount(async () => { if (build) { // Launch the VM if we pass all the prerequisites, otherwise we will show the empty screen with content / information checks. - vmLaunchPrereqs = await bootcClient.checkVMLaunchPrereqs(build); + vmLaunchPrereqs = await bootcClient.checkVMLaunchPrereqs(build.id); if (!vmLaunchPrereqs) { await launchVM(build); diff --git a/packages/frontend/src/lib/disk-image/DiskImagesList.spec.ts b/packages/frontend/src/lib/disk-image/DiskImagesList.spec.ts index 0ffa6cfb..91a4b922 100644 --- a/packages/frontend/src/lib/disk-image/DiskImagesList.spec.ts +++ b/packages/frontend/src/lib/disk-image/DiskImagesList.spec.ts @@ -113,6 +113,7 @@ test('Test clicking on delete button', async () => { deleteButton.click(); expect(spyOnDelete).toHaveBeenCalled(); + expect(spyOnDelete).toHaveBeenCalledWith(['name1']); }); test('Test clicking on build button', async () => { diff --git a/packages/frontend/src/lib/disk-image/DiskImagesList.svelte b/packages/frontend/src/lib/disk-image/DiskImagesList.svelte index 82700ef6..17ace7dc 100644 --- a/packages/frontend/src/lib/disk-image/DiskImagesList.svelte +++ b/packages/frontend/src/lib/disk-image/DiskImagesList.svelte @@ -46,7 +46,7 @@ onMount(() => { let bulkDeleteInProgress = $state(false); async function deleteSelectedBuilds(): Promise { - const selected = history.filter(history => history.selected); + const selected = history.filter(history => history.selected).map(history => history.id); if (selected.length === 0) { return; } diff --git a/packages/shared/src/BootcAPI.ts b/packages/shared/src/BootcAPI.ts index b844e336..df14265a 100644 --- a/packages/shared/src/BootcAPI.ts +++ b/packages/shared/src/BootcAPI.ts @@ -23,14 +23,14 @@ import type { ExamplesList } from './models/examples'; export abstract class BootcApi { static readonly CHANNEL: string = 'BootcApi'; abstract checkPrereqs(): Promise; - abstract checkVMLaunchPrereqs(build: BootcBuildInfo): Promise; - abstract launchVM(build: BootcBuildInfo): Promise; + abstract checkVMLaunchPrereqs(buildId: string): Promise; + abstract launchVM(buildId: string): Promise; abstract buildExists(folder: string, types: BuildType[]): Promise; abstract buildImage(build: BootcBuildInfo, overwrite?: boolean): Promise; abstract pullImage(image: string, arch?: string): Promise; abstract inspectImage(image: ImageInfo): Promise; abstract inspectManifest(image: ImageInfo): Promise; - abstract deleteBuilds(builds: BootcBuildInfo[]): Promise; + abstract deleteBuilds(buildIds: string[]): Promise; abstract selectOutputFolder(): Promise; abstract selectBuildConfigFile(): Promise; abstract listBootcImages(): Promise; From ff75515db3823d3c700fec6dab5416512e3a2135 Mon Sep 17 00:00:00 2001 From: Tim deBoer Date: Tue, 11 Feb 2025 07:44:51 -0500 Subject: [PATCH 2/3] chore: correct throw error, remove console.log Signed-off-by: Tim deBoer --- packages/backend/src/api-impl.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/backend/src/api-impl.ts b/packages/backend/src/api-impl.ts index 86aebd42..ccdd473e 100644 --- a/packages/backend/src/api-impl.ts +++ b/packages/backend/src/api-impl.ts @@ -57,7 +57,7 @@ export class BootcApiImpl implements BootcApi { async checkVMLaunchPrereqs(buildId: string): Promise { const build = this.history.getHistory().find(build => build.id === buildId); if (!build) { - return Promise.reject(new Error(`Could not find build: ${buildId}`)); + throw new Error(`Could not find build: ${buildId}`); } return createVMManager(build).checkVMLaunchPrereqs(); } @@ -344,7 +344,6 @@ export class BootcApiImpl implements BootcApi { // this method is internal and meant to be used by the API implementation protected async notify(id: string, body: unknown = {}): Promise { // Must pass in an empty body, if it is undefined this fails - console.log('notify frontend: ' + id); await this.webview.postMessage({ id, body, From 236f4b7989875b1e3b5ddcd669bbfb3ab9084be8 Mon Sep 17 00:00:00 2001 From: Tim deBoer Date: Tue, 11 Feb 2025 08:13:32 -0500 Subject: [PATCH 3/3] chore: use !! filter to improve type filtering Signed-off-by: Tim deBoer --- packages/backend/src/api-impl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/api-impl.ts b/packages/backend/src/api-impl.ts index ccdd473e..797ee7dc 100644 --- a/packages/backend/src/api-impl.ts +++ b/packages/backend/src/api-impl.ts @@ -109,7 +109,7 @@ export class BootcApiImpl implements BootcApi { // create an array of builds. invalid build ids are ignored const builds = buildIds .map(id => this.history.getHistory().find(build => build.id === id)) - .filter(build => build) as BootcBuildInfo[]; + .filter(build => !!build); // Map each build to a delete operation promise const deletePromises = builds.map(build => this.deleteBuildContainer(build));