From 48c42d243e04aca650ad5536106bddc6ec795e92 Mon Sep 17 00:00:00 2001 From: Flavio F Lima Date: Tue, 4 Mar 2025 20:49:49 +0000 Subject: [PATCH 1/7] feat: implement safeRemoveDirectory function with retry logic for directory removal --- src/backend/storeManagers/hyperplay/utils.ts | 103 ++++++++++++++++++- 1 file changed, 101 insertions(+), 2 deletions(-) diff --git a/src/backend/storeManagers/hyperplay/utils.ts b/src/backend/storeManagers/hyperplay/utils.ts index 7a163f2650..4a31e00d2b 100644 --- a/src/backend/storeManagers/hyperplay/utils.ts +++ b/src/backend/storeManagers/hyperplay/utils.ts @@ -16,9 +16,9 @@ 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' @@ -419,3 +419,102 @@ 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 { + 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) + + // Import fs promises for async operations only when needed + const fsPromises = await import('fs/promises') + + 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 fsPromises.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 fsPromises.access(directory) + // If we get here, directory still exists + logWarning( + `Failed to remove directory ${directory} on attempt ${attempt}/${maxRetries}, directory still exists`, + LogPrefix.HyperPlay + ) + } 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 +} From 2f88722f699a8bab2fb60f30070f2b5cc218cc5f Mon Sep 17 00:00:00 2001 From: Flavio F Lima Date: Tue, 4 Mar 2025 20:50:01 +0000 Subject: [PATCH 2/7] feat: refactor cleanUpDownload to use safeRemoveDirectory and update related async calls --- src/backend/storeManagers/hyperplay/games.ts | 41 ++++++++++---------- src/backend/utils.ts | 2 +- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/backend/storeManagers/hyperplay/games.ts b/src/backend/storeManagers/hyperplay/games.ts index 48c918e1dd..029e429a2d 100644 --- a/src/backend/storeManagers/hyperplay/games.ts +++ b/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 => { 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,13 +1191,13 @@ 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') @@ -1204,7 +1205,7 @@ export async function extract( 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,10 @@ async function applyPatching( if (signal.aborted) { logInfo(`Patching ${appName} aborted`, LogPrefix.HyperPlay) - rmSync(datastoreDir, { recursive: true }) - return { status: 'abort' } - } - - signal.onabort = () => { + await safeRemoveDirectory(datastoreDir, { + sizeThresholdMB: blockSize * totalBlocks + }) aborted = true - rmSync(datastoreDir, { recursive: true }) return { status: 'abort' } } @@ -2005,7 +2003,9 @@ async function applyPatching( } // need this to cover 100% of abort cases if (aborted) { - rmSync(datastoreDir, { recursive: true }) + await safeRemoveDirectory(datastoreDir, { + sizeThresholdMB: blockSize * totalBlocks + }) return { status: 'abort' } } @@ -2020,8 +2020,9 @@ async function applyPatching( }) logInfo(`Patching ${appName} completed`, LogPrefix.HyperPlay) - rmSync(datastoreDir, { recursive: true }) - + await safeRemoveDirectory(datastoreDir, { + sizeThresholdMB: blockSize * totalBlocks + }) return { status: 'done' } } catch (error) { if (error instanceof PatchingError) { @@ -2061,7 +2062,7 @@ async function applyPatching( // errors can be thrown before datastore dir created. rmSync on nonexistent dir blocks indefinitely if (existsSync(datastoreDir)) { - rmSync(datastoreDir, { recursive: true }) + await safeRemoveDirectory(datastoreDir) } return { status: 'error', error: `Error while patching ${error}` } diff --git a/src/backend/utils.ts b/src/backend/utils.ts index db653c1ac2..3558ec5a67 100644 --- a/src/backend/utils.ts +++ b/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 ): Promise { let lastProgressUpdateTime = Date.now() let lastBytesWritten = 0 From fbdc997d14eca7038fbbb6da2099ffd03df0785d Mon Sep 17 00:00:00 2001 From: Flavio F Lima Date: Tue, 4 Mar 2025 21:12:06 +0000 Subject: [PATCH 3/7] chore: re-add second abort check --- src/backend/storeManagers/hyperplay/games.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/backend/storeManagers/hyperplay/games.ts b/src/backend/storeManagers/hyperplay/games.ts index 029e429a2d..dfa50f5246 100644 --- a/src/backend/storeManagers/hyperplay/games.ts +++ b/src/backend/storeManagers/hyperplay/games.ts @@ -1922,6 +1922,14 @@ async function applyPatching( return { status: 'abort' } } + signal.onabort = async () => { + aborted = true + await safeRemoveDirectory(datastoreDir, { + sizeThresholdMB: blockSize * totalBlocks + }) + return { status: 'abort' } + } + for await (const output of generator) { logInfo(output, LogPrefix.HyperPlay) From 273f50305f5b3bd9fd3ffc89de1dc9e154bc4b27 Mon Sep 17 00:00:00 2001 From: Flavio F Lima Date: Tue, 4 Mar 2025 21:31:40 +0000 Subject: [PATCH 4/7] feat: add patching aborted and cleanup failed event tracking --- src/backend/metrics/types.ts | 25 +++++++ src/backend/storeManagers/hyperplay/games.ts | 73 ++++++++++++++++++-- 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/src/backend/metrics/types.ts b/src/backend/metrics/types.ts index 0bc504157d..19a1d9f335 100644 --- a/src/backend/metrics/types.ts +++ b/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 + platform_arch: InstallPlatform + } + sensitiveProperties?: never +} + +export interface PatchingCleanupFailed { + event: 'Patching Cleanup Failed' + properties: { + game_name: string + error: string + game_title: string + platform?: ReturnType + 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 diff --git a/src/backend/storeManagers/hyperplay/games.ts b/src/backend/storeManagers/hyperplay/games.ts index dfa50f5246..159d67d9fa 100644 --- a/src/backend/storeManagers/hyperplay/games.ts +++ b/src/backend/storeManagers/hyperplay/games.ts @@ -2011,8 +2011,35 @@ async function applyPatching( } // need this to cover 100% of abort cases if (aborted) { - await safeRemoveDirectory(datastoreDir, { - sizeThresholdMB: blockSize * totalBlocks + 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' } } @@ -2028,9 +2055,27 @@ async function applyPatching( }) logInfo(`Patching ${appName} completed`, LogPrefix.HyperPlay) - await safeRemoveDirectory(datastoreDir, { - sizeThresholdMB: blockSize * totalBlocks - }) + 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) { @@ -2070,7 +2115,23 @@ async function applyPatching( // errors can be thrown before datastore dir created. rmSync on nonexistent dir blocks indefinitely if (existsSync(datastoreDir)) { - await safeRemoveDirectory(datastoreDir) + 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}` } From 06bc73836152bfee44f3a3162d46a533ac21349e Mon Sep 17 00:00:00 2001 From: Flavio F Lima Date: Thu, 6 Mar 2025 14:52:47 +0000 Subject: [PATCH 5/7] feat: add Sentry error tracking for failed directory removal attempts in safeRemoveDirectory --- src/backend/storeManagers/hyperplay/utils.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/backend/storeManagers/hyperplay/utils.ts b/src/backend/storeManagers/hyperplay/utils.ts index 4a31e00d2b..6c90928142 100644 --- a/src/backend/storeManagers/hyperplay/utils.ts +++ b/src/backend/storeManagers/hyperplay/utils.ts @@ -22,6 +22,7 @@ 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' export async function getHyperPlayStoreRelease( appName: string @@ -486,6 +487,16 @@ export async function safeRemoveDirectory( `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( From 8243d2d3af705718e40bcea00ceaa9dfe1bbe5af Mon Sep 17 00:00:00 2001 From: Flavio F Lima Date: Thu, 6 Mar 2025 14:52:57 +0000 Subject: [PATCH 6/7] chore: bump version to 0.24.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 07a63f3c5e..058c6e113e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "hyperplay", - "version": "0.23.2", + "version": "0.24.0", "private": true, "main": "build/main/main.js", "productName": "HyperPlay", From fe4d25091080608646eb0f3cc4cf7816af348ed1 Mon Sep 17 00:00:00 2001 From: Flavio F Lima Date: Fri, 21 Mar 2025 11:11:12 +0000 Subject: [PATCH 7/7] feat: optimize safeRemoveDirectory by importing fs/promises directly and simplifying removal logic --- src/backend/storeManagers/hyperplay/utils.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/backend/storeManagers/hyperplay/utils.ts b/src/backend/storeManagers/hyperplay/utils.ts index 6c90928142..fcab3b0359 100644 --- a/src/backend/storeManagers/hyperplay/utils.ts +++ b/src/backend/storeManagers/hyperplay/utils.ts @@ -23,6 +23,7 @@ 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 @@ -452,16 +453,13 @@ export async function safeRemoveDirectory( // Log start of removal process logInfo(`Starting removal of directory ${directory}`, LogPrefix.HyperPlay) - // Import fs promises for async operations only when needed - const fsPromises = await import('fs/promises') - 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 fsPromises.rm(directory, { + await rm(directory, { recursive: true, force: true, maxRetries: 3 @@ -481,7 +479,7 @@ export async function safeRemoveDirectory( // Verify directory was actually removed try { - await fsPromises.access(directory) + await access(directory) // If we get here, directory still exists logWarning( `Failed to remove directory ${directory} on attempt ${attempt}/${maxRetries}, directory still exists`,