Skip to content

fix: individual yml download limit#137

Open
Amigache wants to merge 7 commits intoEdgeTX:mainfrom
Amigache:fix/individual-yml-download-limit
Open

fix: individual yml download limit#137
Amigache wants to merge 7 commits intoEdgeTX:mainfrom
Amigache:fix/individual-yml-download-limit

Conversation

@Amigache
Copy link
Copy Markdown
Contributor

This pull request improves the backup download flow to address issues with downloading a large number of files simultaneously, and adds a test to ensure robust handling of bulk downloads. The main changes are grouped into improvements to the download logic and enhancements to test coverage.

Improvements to backup download logic:

  • Changed the file download process in BackupCreateFlow.tsx to download files sequentially with a delay, preventing Chromium's rapid download blocking when many files are downloaded at once. [1] [2]
  • Updated the download handler to use an asynchronous chain, ensuring that each file download is spaced out by 200ms.
  • Modified the promise handler to be asynchronous in order to support sequential downloading.

Test coverage enhancements:

  • Added a test in backup.spec.ts to verify that downloading a large number of models (e.g., 43) works correctly and all files are present and valid.

Bug reported on:
https://discord.com/channels/761870396085108757/854744834698379274/1481212834766655578

…nload limit

When downloading individual .yml backup files, all downloads were triggered
simultaneously via forEach + link.click(). Chromium blocks rapid programmatic
downloads after ~10, causing only model1* files to be saved.

Changed to sequential downloads with a 200ms delay between each file.
Added test verifying all 43 models are returned correctly.
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 11, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 35.71%. Comparing base (7433e52) to head (594eaa2).

Files with missing lines Patch % Lines
src/renderer/pages/backup/BackupCreateFlow.tsx 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #137      +/-   ##
==========================================
+ Coverage   34.95%   35.71%   +0.75%     
==========================================
  Files         115      115              
  Lines        4288     4281       -7     
  Branches     1033     1034       +1     
==========================================
+ Hits         1499     1529      +30     
+ Misses       2585     2542      -43     
- Partials      204      210       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

As suggested in code review:
- Replace manual Blob/link DOM manipulation with legacyDownload (js-file-download)
- Use p-limit(5) for concurrency limiting instead of fully sequential reduce chain
- Use delay() utility from shared/tools instead of raw setTimeout
- Simplify test by mocking legacyDownload directly
freshollie
freshollie previously approved these changes Mar 16, 2026
Copy link
Copy Markdown
Collaborator

@freshollie freshollie left a comment

Choose a reason for hiding this comment

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

If you tested as working, feel free to merge

@Amigache
Copy link
Copy Markdown
Contributor Author

If you tested as working, feel free to merge

After several tests, I discovered that the batches weren't always running correctly. I've made a small change that seems to be effective; images attached.

image image

@freshollie
Copy link
Copy Markdown
Collaborator

Oh btw you could in theory add an e2e for this, but I'm not sure how you setup this sort of test :)

@Amigache
Copy link
Copy Markdown
Contributor Author

Oh btw you could in theory add an e2e for this, but I'm not sure how you setup this sort of test :)

As I already mentioned, this is a very new stack for me, and I'm doing what I can, XD!! But I'm learning little by little, thanks for your time!

@pfeerick pfeerick self-assigned this Mar 18, 2026
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.

3 participants