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

feat: refactor memory and disk storage classes, improve error handling and file size validation #1276

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kalebalebachew
Copy link

  • Refactored DiskStorage and MemoryStorage to use ES6 class syntax for better structure and maintainability.
  • Introduced maxSize option in DiskStorage to enforce file size limits, with proper stream destruction on overflow.
  • Promisified key file operations (randomBytes, unlink) to utilize async/await for cleaner and more readable code.
  • Enhanced error handling in both file.stream and outStream to ensure robust error management.
  • Improved variable declarations by using const for immutable values for better code consistency and clarity.

…ze validation

- Refactored DiskStorage to use async/await for better readability and error handling.
- Added file size validation in DiskStorage with a configurable maxSize option.
- Improved stream error handling in both file.stream and outStream.
- Ensured proper resource cleanup by destroying streams if file size exceeds the limit.
- Updated MemoryStorage for consistent error handling and robustness.
@IamLizu IamLizu requested a review from a team September 14, 2024 12:20
package-lock.json Outdated Show resolved Hide resolved
storage/disk.js Outdated
class DiskStorage {
constructor (opts = {}) {
this.getFilename = opts.filename || getFilename
this.maxSize = opts.maxSize || Infinity
Copy link
Member

Choose a reason for hiding this comment

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

There seems to be a trailing space at the end and standard is unable to fix it automatically. Please remove it.

@kalebalebachew
Copy link
Author

I think it should be fixed by now. let me know what to improve if u have one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants