Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

flavioislima
Copy link
Contributor

@flavioislima flavioislima commented Mar 4, 2025

This pull request introduces several changes to the src/backend/storeManagers/hyperplay/games.ts and related files to improve the handling of directory removal and ensure asynchronous operations are properly awaited. The most important changes include the addition of a new utility function safeRemoveDirectory, modifications to existing functions to use this new utility, and updates to import statements.

Key Changes:

Utility Enhancements:

  • Added a new utility function safeRemoveDirectory in src/backend/storeManagers/hyperplay/utils.ts to safely remove directories with retry logic and handle Windows EPERM issues. This function must always be awaited to prevent race conditions.

Function Modifications:

  • Updated cleanUpDownload function in src/backend/storeManagers/hyperplay/games.ts to be asynchronous and use safeRemoveDirectory instead of rmSync.
  • Modified the onCancel function in downloadGame to be asynchronous and await cleanUpDownload.
  • Updated the extract function to await cleanUpDownload in multiple places to ensure proper cleanup. [1] [2] [3]

Directory Removal:

  • Replaced rmSync with safeRemoveDirectory in the applyPatching function to handle directory removal asynchronously and with retry logic. [1] [2] [3] [4]

Import Updates:

  • Added necessary imports for safeRemoveDirectory and updated existing imports to include logWarning and rmSync in src/backend/storeManagers/hyperplay/utils.ts.

How to test

https://app.qase.io/project/HC?case=571&suite=39


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@flavioislima flavioislima changed the title feat: refactor cleanUpDownload to use safeRemoveDirectory and update related async calls [TECH] Improve Patching process to avoid file and directory removal errors Mar 4, 2025
@flavioislima flavioislima added the PR: Ready-For-Review PR is ready to be reviewed by peers label Mar 4, 2025
@BrettCleary
Copy link
Collaborator

Can we create a test plan for this? @flavioislima @nyghtstalker

Copy link
Collaborator

@BrettCleary BrettCleary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. let's test it out 🚀

Could you bump the package.json version on this branch too?

@flavioislima
Copy link
Contributor Author

Can we create a test plan for this? @flavioislima @nyghtstalker

I wrote a few QASE tests for the patching that should cover those. I will just update with the expected behavior of having all .temp files deleted.

@flavioislima flavioislima added PR: Ready-For-Test PR is ready to be tested by a QA and removed PR: Ready-For-Review PR is ready to be reviewed by peers labels Mar 6, 2025
Copy link

@nyghtstalker nyghtstalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Confirmed the game patches as expected
  • Confirmed the contents of the .temp folder are deleted when canceling
  • Confirmed the contents of the .temp folder are deleted after update is complete
  • https://app.qase.io/case/HC-571

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Ready-For-Test PR is ready to be tested by a QA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants