Clearer error boundary, clean up some naming

Author: franknoirot

interface.d.ts

@@ -65,7 +65,10 @@ export interface IElectronAPI {
   sep: typeof path.sep
   copy: typeof fs.cp
   // No such thing as fs.mv, but our function will use fs.cp as a fallback
-  move: (source: string | URL, destination: string | URL) => Promise<void>
+  move: (
+    source: string | URL,
+    destination: string | URL
+  ) => Promise<void | Error>
   rename: (prev: string, next: string) => ReturnType<typeof fs.rename>
   packageJson: {
     name: string
@@ -97,7 +100,7 @@ export interface IElectronAPI {
     callback: (value: { version: string; releaseNotes: string }) => void
   ) => Electron.IpcRenderer
   onUpdateError: (callback: (value: { error: Error }) => void) => Electron
-  getArchivePath: () => Promise<string>
+  getPathUserData: () => Promise<string>
   appRestart: () => void
   appCheckForUpdates: () => Promise<unknown>
   getArgvParsed: () => any

src/lib/paths.ts

@@ -324,13 +324,17 @@ export function getFilePathRelativeToProject(
   return absoluteFilePath.slice(projectIndexInPath + sliceOffset) ?? ''
 }
 
+/** Gets our "archive" directory for deletion/restoration, within the electron userData path */
 async function getArchiveBasePath() {
-  return window.electron ? window.electron.getArchivePath() : `/${ARCHIVE_DIR}`
+  return `${window.electron ? await window.electron.getPathUserData() : '/'}'${ARCHIVE_DIR}`
 }
 
+/**
+ * Our archive simply mirrors the absolute location on disk for the item
+ * being archived, joining it with the archive base path.
+ */
 export async function toArchivePath(absolutePath: string) {
   const basePath = await getArchiveBasePath()
-  console.log('basePath', basePath)
   return window.electron
     ? window.electron.join(basePath, absolutePath)
     : webSafeJoin([basePath, absolutePath])

src/machines/systemIO/systemIOMachineDesktop.ts

@@ -20,6 +20,7 @@ import {
   parentPathRelativeToProject,
 } from '@src/lib/paths'
 import type { Project } from '@src/lib/project'
+import { isErr } from '@src/lib/trap'
 import type { AppMachineContext } from '@src/lib/types'
 import type { ModuleType } from '@src/lib/wasm_lib_wrapper'
 import { systemIOMachine } from '@src/machines/systemIO/systemIOMachine'
@@ -762,8 +763,23 @@ export const systemIOMachineDesktop = systemIOMachine.provide({
         }
       }) => {
         if (window.electron) {
-          console.log('moving', input)
-          await window.electron.move(input.src, input.target)
+          const result = await window.electron.move(input.src, input.target)
+          if (
+            isErr(result) &&
+            // Where did the error code go? It's available in preload.ts
+            result.message.includes('ENOTEMPTY') &&
+            window.electron
+          ) {
+            // TODO: this force deletion behavior assumes this move is only
+            // really used in our archive/restore workflow. We should make
+            // dedicated archive/restore code paths for that if we need cases
+            // where we want to check with the user before going through with forcing.
+            await window.electron.rm(input.target, {
+              recursive: true,
+              force: true,
+            })
+            await window.electron.move(input.src, input.target)
+          }
           return {
             message: input.successMessage || 'Moved successfully',
             requestedAbsolutePath: '',

src/main.ts

@@ -29,7 +29,6 @@ import {
 } from '@src/commandLineArgs'
 import { initialiseWasmNode } from '@src/lang/wasmUtilsNode'
 import {
-  ARCHIVE_DIR,
   OAUTH2_DEVICE_CLIENT_ID,
   ZOO_STUDIO_PROTOCOL,
 } from '@src/lib/constants'
@@ -524,9 +523,7 @@ ipcMain.handle('disable-menu', (event, data) => {
   disableMenu(menuId)
 })
 
-ipcMain.handle('get-archive-path', () =>
-  path.join(app.getPath('userData'), ARCHIVE_DIR)
-)
+ipcMain.handle('get-path-userdata', () => app.getPath('userData'))
 
 app.on('ready', () => {
   // Disable auto updater on non-versioned builds

src/preload.ts

@@ -129,6 +129,7 @@ async function move(source: string | URL, destination: string | URL) {
     .rename(source, destination)
     .catch(async (e) => {
       if (e.code === 'ENOENT') {
+        // We need to make the directories in the destination
         const dirToMake =
           type === 'dir'
             ? destination
@@ -143,6 +144,7 @@ async function move(source: string | URL, destination: string | URL) {
     .catch((e) => {
       console.error(e)
       if (e.code === 'EXDEV') {
+        // Rename somehow isn't allowed, but copy+delete might be 🤞
         return Promise.all([
           fs.cp(source, destination, {
             recursive: true,
@@ -151,15 +153,9 @@ async function move(source: string | URL, destination: string | URL) {
             recursive: true,
           }),
         ])
-      } else if (e.code === 'ENOTEMPTY') {
-        return Promise.all([
-          fs.rm(destination, { recursive: true, force: true }),
-          fs.rename(source, destination),
-        ])
       }
       return e
     })
-    .catch((e) => e)
 }
 
 // Electron has behavior where it doesn't clone the prototype chain over.
@@ -266,9 +262,9 @@ const disableMenu = async (menuId: string): Promise<any> => {
   })
 }
 
-// Get the user data folder according to Electron, plus our designated archive folder
-const getArchivePath = async (): Promise<string> =>
-  ipcRenderer.invoke('get-archive-path')
+// Get the user data folder according to Electron
+const getPathUserData = async (): Promise<string> =>
+  ipcRenderer.invoke('get-path-userdata')
 
 /**
  * Gotcha: Even if the callback function is the same function in JS memory
@@ -366,5 +362,5 @@ contextBridge.exposeInMainWorld('electron', {
   canReadWriteDirectory,
   copy,
   move,
-  getArchivePath,
+  getPathUserData,
 })