[TECH] Improve Patching process to avoid file and directory removal errors

package.json

@@ -1,6 +1,6 @@
 {
   "name": "hyperplay",
-  "version": "0.23.3",
+  "version": "0.24.0",
   "private": true,
   "main": "build/main/main.js",
   "homepage": "./",

src/backend/metrics/types.ts

@@ -228,6 +228,29 @@ export interface PatchingFailed {
   sensitiveProperties?: never
 }
 
+export interface PatchingAborted {
+  event: 'Patching Aborted'
+  properties: {
+    game_name: string
+    game_title: string
+    platform: ReturnType<typeof getPlatformName>
+    platform_arch: InstallPlatform
+  }
+  sensitiveProperties?: never
+}
+
+export interface PatchingCleanupFailed {
+  event: 'Patching Cleanup Failed'
+  properties: {
+    game_name: string
+    error: string
+    game_title: string
+    platform?: ReturnType<typeof getPlatformName>
+    platform_arch?: InstallPlatform
+  }
+  sensitiveProperties?: never
+}
+
 export interface PatchingTooSlow {
   event: 'Patching Too Slow'
   properties: {
@@ -475,6 +498,8 @@ export type PossibleMetricPayloads =
   | PatchingStarted
   | PatchingSuccess
   | PatchingFailed
+  | PatchingAborted
+  | PatchingCleanupFailed
   | PatchingTooSlow
   | AccountDropdownPortfolioClicked
 

src/backend/storeManagers/hyperplay/games.ts

@@ -65,7 +65,8 @@ import {
   handleArchAndPlatform,
   handlePlatformReversed,
   runModPatcher,
-  sanitizeVersion
+  sanitizeVersion,
+  safeRemoveDirectory
 } from './utils'
 import { getSettings as getSettingsSideload } from 'backend/storeManagers/sideload/games'
 import {
@@ -423,11 +424,11 @@ const findExecutables = async (folderPath: string): Promise<string[]> => {
   return executables
 }
 
-export function cleanUpDownload(appName: string, directory: string) {
+export async function cleanUpDownload(appName: string, directory: string) {
   inProgressDownloadsMap.delete(appName)
   inProgressExtractionsMap.delete(appName)
   deleteAbortController(appName)
-  rmSync(directory, { recursive: true, force: true })
+  await safeRemoveDirectory(directory)
 }
 
 function getDownloadUrl(platformInfo: PlatformConfig, appName: string) {
@@ -523,9 +524,9 @@ async function downloadGame(
       res()
     }
 
-    function onCancel() {
+    async function onCancel() {
       try {
-        cleanUpDownload(appName, directory)
+        await cleanUpDownload(appName, directory)
       } catch (err) {
         rej(err)
       }
@@ -1181,7 +1182,7 @@ export async function extract(
             body: `Installed`
           })
 
-          cleanUpDownload(appName, directory)
+          await cleanUpDownload(appName, directory)
 
           sendFrontendMessage('refreshLibrary', 'hyperplay')
 
@@ -1190,21 +1191,21 @@ export async function extract(
           })
         }
       )
-      extractService.once('error', (error: Error) => {
+      extractService.once('error', async (error: Error) => {
         logError(`Extracting Error ${error.message}`, LogPrefix.HyperPlay)
 
         cancelQueueExtraction()
         callAbortController(appName)
 
-        cleanUpDownload(appName, directory)
+        await cleanUpDownload(appName, directory)
 
         sendFrontendMessage('refreshLibrary', 'hyperplay')
 
         resolve({
           status: 'error'
         })
       })
-      extractService.once('canceled', () => {
+      extractService.once('canceled', async () => {
         logInfo(
           `Canceled Extracting: Cancellation completed on ${appName} - Destination ${destinationPath}`,
           LogPrefix.HyperPlay
@@ -1242,7 +1243,7 @@ export async function extract(
           body: 'Installation Stopped'
         })
 
-        cleanUpDownload(appName, directory)
+        await cleanUpDownload(appName, directory)
 
         sendFrontendMessage('refreshLibrary', 'hyperplay')
 
@@ -1914,13 +1915,18 @@ async function applyPatching(
 
     if (signal.aborted) {
       logInfo(`Patching ${appName} aborted`, LogPrefix.HyperPlay)
-      rmSync(datastoreDir, { recursive: true })
+      await safeRemoveDirectory(datastoreDir, {
+        sizeThresholdMB: blockSize * totalBlocks
+      })
+      aborted = true
       return { status: 'abort' }
     }
 
-    signal.onabort = () => {
+    signal.onabort = async () => {
       aborted = true
-      rmSync(datastoreDir, { recursive: true })
+      await safeRemoveDirectory(datastoreDir, {
+        sizeThresholdMB: blockSize * totalBlocks
+      })
       return { status: 'abort' }
     }
 
@@ -2005,7 +2011,36 @@ async function applyPatching(
     }
     // need this to cover 100% of abort cases
     if (aborted) {
-      rmSync(datastoreDir, { recursive: true })
+      try {
+        await safeRemoveDirectory(datastoreDir, {
+          sizeThresholdMB: blockSize * totalBlocks
+        })
+      } catch (cleanupError) {
+        trackEvent({
+          event: 'Patching Cleanup Failed',
+          properties: {
+            error: `${cleanupError}`,
+            game_name: gameInfo.app_name,
+            game_title: gameInfo.title,
+            platform: getPlatformName(platform),
+            platform_arch: platform
+          }
+        })
+
+        logWarning(
+          `Patching aborted and cleanup failed: ${cleanupError}`,
+          LogPrefix.HyperPlay
+        )
+      }
+      trackEvent({
+        event: 'Patching Aborted',
+        properties: {
+          game_name: gameInfo.app_name,
+          game_title: gameInfo.title,
+          platform: getPlatformName(platform),
+          platform_arch: platform
+        }
+      })
       return { status: 'abort' }
     }
 
@@ -2020,8 +2055,27 @@ async function applyPatching(
     })
 
     logInfo(`Patching ${appName} completed`, LogPrefix.HyperPlay)
-    rmSync(datastoreDir, { recursive: true })
+    try {
+      await safeRemoveDirectory(datastoreDir, {
+        sizeThresholdMB: blockSize * totalBlocks
+      })
+    } catch (cleanupError) {
+      trackEvent({
+        event: 'Patching Cleanup Failed',
+        properties: {
+          error: `${cleanupError}`,
+          game_name: gameInfo.app_name,
+          game_title: gameInfo.title,
+          platform: getPlatformName(platform),
+          platform_arch: platform
+        }
+      })
 
+      logWarning(
+        `Patching succeeded but cleanup failed: ${cleanupError}`,
+        LogPrefix.HyperPlay
+      )
+    }
     return { status: 'done' }
   } catch (error) {
     if (error instanceof PatchingError) {
@@ -2061,7 +2115,23 @@ async function applyPatching(
 
     // errors can be thrown before datastore dir created. rmSync on nonexistent dir blocks indefinitely
     if (existsSync(datastoreDir)) {
-      rmSync(datastoreDir, { recursive: true })
+      try {
+        await safeRemoveDirectory(datastoreDir)
+      } catch (cleanupError) {
+        trackEvent({
+          event: 'Patching Cleanup Failed',
+          properties: {
+            error: `${cleanupError}`,
+            game_name: gameInfo.app_name,
+            game_title: gameInfo.title
+          }
+        })
+
+        logWarning(
+          `Patching failed and cleanup failed: ${cleanupError}`,
+          LogPrefix.HyperPlay
+        )
+      }
     }
 
     return { status: 'error', error: `Error while patching ${error}` }

src/backend/storeManagers/hyperplay/utils.ts

@@ -16,12 +16,14 @@ import {
   valistListingsApiUrl
 } from 'backend/constants'
 import { getGameInfo } from './games'
-import { LogPrefix, logError, logInfo } from 'backend/logger/logger'
+import { LogPrefix, logError, logInfo, logWarning } from 'backend/logger/logger'
 import { join } from 'path'
-import { existsSync } from 'graceful-fs'
+import { existsSync, rmSync } from 'graceful-fs'
 import { ProjectMetaInterface } from '@valist/sdk/dist/typesShared'
 import getPartitionCookies from 'backend/utils/get_partition_cookies'
 import { DEV_PORTAL_URL } from 'common/constants'
+import { captureException } from '@sentry/electron'
+import { access, rm } from 'fs/promises'
 
 export async function getHyperPlayStoreRelease(
   appName: string
@@ -419,3 +421,109 @@ export const runModPatcher = async (appName: string) => {
     throw new Error(`Error running patcher: ${error}`)
   }
 }
+
+type SafeRemoveDirectoryOptions = {
+  maxRetries?: number
+  retryDelay?: number
+  sizeThresholdMB?: number
+}
+
+/**
+ * Safely removes a directory with retry logic to handle Windows EPERM issues
+ * @param directory Path to the directory to remove
+ * @param options Optional configuration for removal
+ * @param options.maxRetries Maximum number of removal attempts before giving up (default: 5)
+ * @param options.retryDelay Delay in milliseconds between removal attempts (default: 10000)
+ * @param options.sizeThresholdMB Threshold in MB above which async removal is used (default: 500)
+ * @returns True if directory was successfully removed or doesn't exist, false otherwise
+ * @warning This function MUST always be awaited to prevent race conditions
+ */
+export async function safeRemoveDirectory(
+  directory: string,
+  {
+    maxRetries = 10,
+    retryDelay = 6000,
+    sizeThresholdMB = 500
+  }: SafeRemoveDirectoryOptions = {}
+): Promise<boolean> {
+  if (!existsSync(directory)) {
+    return true // Directory doesn't exist, nothing to remove
+  }
+
+  // Log start of removal process
+  logInfo(`Starting removal of directory ${directory}`, LogPrefix.HyperPlay)
+
+  for (let attempt = 1; attempt <= maxRetries; attempt++) {
+    try {
+      // Use different removal strategies based on expected size
+      // For directories larger than threshold, use async removal to not block the main thread
+      if (sizeThresholdMB > 250) {
+        try {
+          await rm(directory, {
+            recursive: true,
+            force: true,
+            maxRetries: 3
+          })
+        } catch (asyncError) {
+          // Fallback to sync if async fails
+          logWarning(
+            `Async removal failed, falling back to sync removal: ${asyncError}`,
+            LogPrefix.HyperPlay
+          )
+          rmSync(directory, { recursive: true, force: true, maxRetries: 3 })
+        }
+      } else {
+        // For smaller directories, use sync removal
+        rmSync(directory, { recursive: true, force: true, maxRetries: 3 })
+      }
+
+      // Verify directory was actually removed
+      try {
+        await access(directory)
+        // If we get here, directory still exists
+        logWarning(
+          `Failed to remove directory ${directory} on attempt ${attempt}/${maxRetries}, directory still exists`,
+          LogPrefix.HyperPlay
+        )
+        captureException(
+          new Error(`Failed to remove directory, directory still exists`),
+          {
+            extra: {
+              directory,
+              attempts: attempt,
+              method: 'safeRemoveDirectory'
+            }
+          }
+        )
+      } catch {
+        // Directory doesn't exist (access threw an error), removal succeeded
+        logInfo(
+          `Successfully removed directory ${directory}`,
+          LogPrefix.HyperPlay
+        )
+        return true
+      }
+    } catch (error) {
+      logWarning(
+        `Error removing directory ${directory} on attempt ${attempt}/${maxRetries}: ${error}`,
+        LogPrefix.HyperPlay
+      )
+    }
+
+    // Use exponential backoff for retries (increases wait time with each attempt)
+    if (attempt < maxRetries) {
+      const backoffDelay = retryDelay * Math.pow(1.5, attempt - 1)
+      logInfo(
+        `Waiting ${backoffDelay}ms before retry ${attempt + 1}/${maxRetries}`,
+        LogPrefix.HyperPlay
+      )
+      await new Promise((resolve) => setTimeout(resolve, backoffDelay))
+    }
+  }
+
+  logError(
+    `Failed to remove directory ${directory} after ${maxRetries} attempts`,
+    LogPrefix.HyperPlay
+  )
+  return false
+}

src/backend/utils.ts

@@ -983,7 +983,7 @@ export async function downloadFile(
   abortController: AbortController,
   progressCallback?: ProgressCallback,
   onCompleted?: (file: File) => void,
-  onCancel?: (item: DownloadItem) => void
+  onCancel?: (item: DownloadItem) => Promise<void>
 ): Promise<DownloadItem> {
   let lastProgressUpdateTime = Date.now()
   let lastBytesWritten = 0