fix: add error handling and temp file cleanup in PNG optimization #256
Conversation
This PR improves the reliability of PNG image optimization by adding proper error handling and temporary file cleanup. Changes made: - Added try-catch block around PNG optimization logic - Implemented cleanup of temporary files in case of errors - Ensures temporary files are not left on disk even if optimization fails - Maintains original error propagation for proper error reporting This change prevents potential disk space leaks and improves the robustness of the image optimization process.
WalkthroughThe pull request introduces type safety and import refinements to the Changes
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
_api/src/optimizeImages.ts (1)
18-18: Consider consolidating the size limit checks.The file size checking and deletion logic is duplicated for PNG and SVG files. Consider extracting this into a reusable function.
+interface SizeLimit { + extension: string; + limit: number; +} + +function handleOversizedFile(file: string, size: number, sizeLimit: SizeLimit, dir: string): void { + if (size > sizeLimit.limit) { + const relativePath = relative(dir, file); + console.warn(chalk.yellow(`Deleting oversized ${sizeLimit.extension}: ${relativePath}`)); + unlinkSync(file); + } +} export async function optimizeImages(dir: string): Promise<void> { // ... existing code ... const newSize = getFileSize(file) const relativePath = relative(dir, file) console.log(`Optimized ${relativePath}: ${originalSize} bytes -> ${newSize} bytes`) - if (ext === ".svg" && newSize > SVG_SIZE_LIMIT) { - console.warn(chalk.yellow(`Deleting oversized SVG: ${relativePath}`)) - unlinkSync(file) - } - - if (ext === ".png" && newSize > PNG_SIZE_LIMIT) { - console.warn(chalk.yellow(`Deleting oversized PNG: ${relativePath}`)) - unlinkSync(file) - } + handleOversizedFile(file, newSize, { extension: 'SVG', limit: SVG_SIZE_LIMIT }, dir); + handleOversizedFile(file, newSize, { extension: 'PNG', limit: PNG_SIZE_LIMIT }, dir);Also applies to: 47-55
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
_api/src/optimizeImages.ts(4 hunks)
🔇 Additional comments (2)
_api/src/optimizeImages.ts (2)
1-2: Well-structured imports and type safety improvements!Good improvements:
- Specific imports from node modules enhance tree-shaking
- Adding readonly type to imageExtensions prevents accidental modifications
Also applies to: 15-15
61-86: Consider using a more secure temporary file creation method.While using UUID reduces the chance of collisions, there's still a potential race condition between checking if a temporary file exists and creating it. Consider using Node's
fs.mkstempor a similar atomic operation.Also, verify that the sharp library properly handles the PNG optimization:
| function optimizeSvg(filePath: string): void { | ||
| const data = readFileSync(filePath, "utf8") | ||
| const result = optimize(data, { path: filePath }) | ||
| fs.writeFileSync(filePath, result.data) | ||
| writeFileSync(filePath, result.data) |
There was a problem hiding this comment.
Add error handling to SVG optimization.
The SVG optimization lacks error handling for file operations, unlike the PNG optimization. Consider wrapping the operations in a try-catch block.
-function optimizeSvg(filePath: string): void {
- const data = readFileSync(filePath, "utf8")
- const result = optimize(data, { path: filePath })
- writeFileSync(filePath, result.data)
+function optimizeSvg(filePath: string): void {
+ try {
+ const data = readFileSync(filePath, "utf8")
+ const result = optimize(data, { path: filePath })
+ writeFileSync(filePath, result.data)
+ } catch (error) {
+ console.error(`Failed to optimize SVG file: ${filePath}`, error)
+ throw error
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function optimizeSvg(filePath: string): void { | |
| const data = readFileSync(filePath, "utf8") | |
| const result = optimize(data, { path: filePath }) | |
| fs.writeFileSync(filePath, result.data) | |
| writeFileSync(filePath, result.data) | |
| function optimizeSvg(filePath: string): void { | |
| try { | |
| const data = readFileSync(filePath, "utf8") | |
| const result = optimize(data, { path: filePath }) | |
| writeFileSync(filePath, result.data) | |
| } catch (error) { | |
| console.error(`Failed to optimize SVG file: ${filePath}`, error) | |
| throw error | |
| } | |
| } |
This PR improves the reliability of PNG image optimization by adding proper error handling and temporary file cleanup.
Changes made:
This change prevents potential disk space leaks and improves the robustness of the image optimization process.
Summary by CodeRabbit
Refactor
Bug Fixes