Add unzip functionality for non-torrent downloads #1437
Conversation
Add functionality to automatically unzip downloaded .zip files from sources other than torrents. * Add `unzipFile` helper function in `src/main/services/download/helpers.ts` to handle unzipping .zip files. * Modify `src/main/services/download/download-manager.ts` to check if the downloaded file is a .zip file and unzip it after download completion. * Update `src/main/services/python-rpc.ts` to support unzipping functionality by handling .zip files in the `spawn` method.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Added functionality to automatically extract .zip files after download completion, with implementations across download management and helper utilities.
- Potential data loss risk in
download-manager.tswhere source zip and output directory share the same path:path.join(game.downloadPath!, game.folderName!) - Incorrect unzip timing in
python-rpc.ts- attempting extraction inspawn()before download completion - Missing cleanup logic for zip files after successful extraction in
unzipFilehelper - Missing validation for zip file integrity/existence before extraction attempts
- No handling for insufficient disk space scenarios during extraction
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
3 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile
| const zipFilePath = path.join(game.downloadPath!, game.folderName!); | ||
| const outputDir = path.join(game.downloadPath!, game.folderName!); | ||
| await unzipFile(zipFilePath, outputDir); |
There was a problem hiding this comment.
logic: Using the same path for both source zip and output directory could cause issues. The zip file might get overwritten during extraction. Consider using a temporary extraction directory or removing the zip file after successful extraction.
| if (game.uri?.endsWith(".zip")) { | ||
| const zipFilePath = path.join(game.downloadPath!, game.folderName!); | ||
| const outputDir = path.join(game.downloadPath!, game.folderName!); | ||
| await unzipFile(zipFilePath, outputDir); | ||
| } |
There was a problem hiding this comment.
logic: No error handling for the unzip operation. If it fails, the game status will still be marked as complete.
| if (game.uri?.endsWith(".zip")) { | |
| const zipFilePath = path.join(game.downloadPath!, game.folderName!); | |
| const outputDir = path.join(game.downloadPath!, game.folderName!); | |
| await unzipFile(zipFilePath, outputDir); | |
| } | |
| if (game.uri?.endsWith(".zip")) { | |
| try { | |
| const zipFilePath = path.join(game.downloadPath!, game.folderName!); | |
| const outputDir = path.join(game.downloadPath!, game.folderName!); | |
| await unzipFile(zipFilePath, outputDir); | |
| } catch (error) { | |
| logger.error('Failed to extract zip file:', error); | |
| await gameRepository.update({ id: gameId }, { status: 'error' }); | |
| } | |
| } |
| if (initialDownload && initialDownload.url.endsWith(".zip")) { | ||
| const zipFilePath = path.join(initialDownload.save_path, initialDownload.url.split('/').pop()!); | ||
| const outputDir = initialDownload.save_path; | ||
| await unzipFile(zipFilePath, outputDir); | ||
| } |
There was a problem hiding this comment.
logic: Unzipping here assumes the file is already downloaded, but the Python process may not have completed the download yet. This should be moved to a post-download hook or callback.
|
|
||
| // Handle unzipping if initialDownload is a .zip file | ||
| if (initialDownload && initialDownload.url.endsWith(".zip")) { | ||
| const zipFilePath = path.join(initialDownload.save_path, initialDownload.url.split('/').pop()!); |
There was a problem hiding this comment.
logic: Using .pop()! is unsafe - could throw if URL doesn't contain a forward slash. Use path.basename() instead.
| const zipFilePath = path.join(initialDownload.save_path, initialDownload.url.split('/').pop()!); | |
| const zipFilePath = path.join(initialDownload.save_path, path.basename(initialDownload.url)); |
| if (initialDownload && initialDownload.url.endsWith(".zip")) { | ||
| const zipFilePath = path.join(initialDownload.save_path, initialDownload.url.split('/').pop()!); | ||
| const outputDir = initialDownload.save_path; | ||
| await unzipFile(zipFilePath, outputDir); |
There was a problem hiding this comment.
style: Missing try/catch block around unzipFile call. Unhandled promise rejection could crash the app.
| export const unzipFile = async (zipFilePath: string, outputDir: string) => { | ||
| try { | ||
| await fs.createReadStream(zipFilePath) | ||
| .pipe(unzipper.Extract({ path: outputDir })) | ||
| .promise(); |
There was a problem hiding this comment.
logic: No validation that zipFilePath exists or is accessible before attempting to read. Add fs.access() check first.
| export const unzipFile = async (zipFilePath: string, outputDir: string) => { | |
| try { | |
| await fs.createReadStream(zipFilePath) | |
| .pipe(unzipper.Extract({ path: outputDir })) | |
| .promise(); | |
| export const unzipFile = async (zipFilePath: string, outputDir: string) => { | |
| try { | |
| await fs.promises.access(zipFilePath, fs.constants.R_OK); | |
| await fs.createReadStream(zipFilePath) | |
| .pipe(unzipper.Extract({ path: outputDir })) | |
| .promise(); |
| @@ -42,7 +43,7 @@ export class PythonRPC { | |||
| readable.on("data", pythonRpcLogger.log); | |||
| } | |||
|
|
|||
| public static spawn( | |||
| public static async spawn( | |||
There was a problem hiding this comment.
logic: Method marked async but doesn't await Python process startup. Could lead to race conditions.
| export const unzipFile = async (zipFilePath: string, outputDir: string) => { | ||
| try { | ||
| await fs.createReadStream(zipFilePath) | ||
| .pipe(unzipper.Extract({ path: outputDir })) | ||
| .promise(); |
There was a problem hiding this comment.
logic: No validation that outputDir exists and is writable. Create directory if it doesn't exist.
| export const unzipFile = async (zipFilePath: string, outputDir: string) => { | |
| try { | |
| await fs.createReadStream(zipFilePath) | |
| .pipe(unzipper.Extract({ path: outputDir })) | |
| .promise(); | |
| export const unzipFile = async (zipFilePath: string, outputDir: string) => { | |
| try { | |
| await fs.promises.mkdir(outputDir, { recursive: true }); | |
| await fs.createReadStream(zipFilePath) | |
| .pipe(unzipper.Extract({ path: outputDir })) | |
| .promise(); |
Add functionality to automatically unzip downloaded .zip files from sources other than torrents.
unzipFilehelper function insrc/main/services/download/helpers.tsto handle unzipping .zip files.src/main/services/download/download-manager.tsto check if the downloaded file is a .zip file and unzip it after download completion.src/main/services/python-rpc.tsto support unzipping functionality by handling .zip files in thespawnmethod.