fix(file): clean up failed cloud downloads

[lorenzozanee]

Summary

Move the cloud-download temp-file cleanup for S3 and Google Drive out of the NamedTemporaryFile context so the file handle is closed before unlinking on failure.

Tests

• env -u ALL_PROXY -u HTTP_PROXY -u HTTPS_PROXY -u all_proxy -u http_proxy -u https_proxy uv run pytest src/backend/tests/unit/components/files_and_knowledge/test_file_component.py -q
• env -u ALL_PROXY -u HTTP_PROXY -u HTTPS_PROXY -u all_proxy -u http_proxy -u https_proxy uv run ruff check src/lfx/src/lfx/components/files_and_knowledge/file.py src/backend/tests/unit/components/files_and_knowledge/test_file_component.py
• env -u ALL_PROXY -u HTTP_PROXY -u HTTPS_PROXY -u all_proxy -u http_proxy -u https_proxy uv run ruff format src/lfx/src/lfx/components/files_and_knowledge/file.py src/backend/tests/unit/components/files_and_knowledge/test_file_component.py --check

Fixes #14866

Summary by CodeRabbit

• Bug Fixes
  • Improved cleanup of temporary files when cloud-storage downloads fail.
  • S3 and Google Drive download errors now close temporary files before reporting the failure, preventing leftover files.
  • Preserved existing error messages for easier troubleshooting.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 1ce59e8f-6c6c-4ef8-a3fb-b63e29849c92

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: a3ef1159-0550-49ff-84a4-aff88d12ccf0

📥 Commits

Reviewing files that changed from the base of the PR and between e3abffc and 4abceb2.

📒 Files selected for processing (2)

• src/backend/tests/unit/components/files_and_knowledge/test_file_component.py
• src/lfx/src/lfx/components/files_and_knowledge/file.py

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

Walkthrough

The cloud download handlers now close temporary files before raising download errors and removing failed downloads. S3 and Google Drive tests track temporary files directly and verify that cleanup succeeds.

Changes

Cloud download cleanup

Layer / File(s)	Summary
Close files before failure cleanup src/lfx/src/lfx/components/files_and_knowledge/file.py	S3 and Google Drive download exceptions are captured while the temporary file is open. The handlers raise the existing RuntimeError after the file context closes.
Verify deterministic cleanup src/backend/tests/unit/components/files_and_knowledge/test_file_component.py	The failure tests create tracked temporary files, simulate cleanup failure while handles are open, and assert that the files are removed after the expected error.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 4abce

The change closes temporary-file handles before cleanup for failed S3 and Google Drive downloads, reducing cleanup failures without altering public interfaces or deployment behavior. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: erichare

🚥 Pre-merge checks | ✅ 8 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (8 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: cleanup for failed cloud downloads.
Linked Issues check	✅ Passed	The implementation addresses issue #14866 by closing the temporary file before unlinking it on failed S3 and Google Drive downloads, then propagating the original RuntimeError. The updated tests verif…
Out of Scope Changes check	✅ Passed	All changes support issue #14866. The code changes target the two affected cloud-download paths, and the test changes verify their cleanup behavior. No unrelated changes are present.
Test Coverage For New Implementations	✅ Passed	The PR includes targeted regression tests for both changed failure paths: test_s3_temp_file_cleanup_on_download_failure and test_google_drive_temp_file_cleanup_on_download_failure in the conventio…
Test Quality And Coverage	✅ Passed	Tests adequately cover the changed cleanup behavior. The S3 and Google Drive failure tests invoke the real component methods, force a download exception, require the expected RuntimeError, and assert …
Test File Naming And Structure	✅ Passed	The pull request passes the test naming and structure check. The changed backend module is src/backend/tests/unit/components/files_and_knowledge/test_file_component.py, which follows the test_*.py…
Excessive Mock Usage Warning	✅ Passed	PASS. The changed tests do not excessively mock core logic. Each test calls the real FileComponent._read_from_aws_s3() or _read_from_google_drive(), creates a real temporary file under tmp_path,…

Full details: Linked Issues check

Explanation

The implementation addresses issue #14866 by closing the temporary file before unlinking it on failed S3 and Google Drive downloads, then propagating the original RuntimeError. The updated tests verify that the temporary file is removed after each failure.

Full details: Test Coverage For New Implementations

Explanation

The PR includes targeted regression tests for both changed failure paths: test_s3_temp_file_cleanup_on_download_failure and test_google_drive_temp_file_cleanup_on_download_failure in the convention-compliant backend file test_file_component.py. Each test forces a download exception, tracks the created temporary file, simulates unlink failure while the handle is open, and asserts that the file is removed after the RuntimeError. The tests therefore exercise the new post-context cleanup behavior rather than acting as placeholders.

Full details: Test Quality And Coverage

Explanation

Tests adequately cover the changed cleanup behavior. The S3 and Google Drive failure tests invoke the real component methods, force a download exception, require the expected RuntimeError, and assert that the tracked temporary file is removed. Their guarded Path.unlink mock fails while the temporary handle is open, so the tests verify the required close-before-unlink ordering. Existing success-path tests also verify local cleanup for both providers. The changes use pytest patterns, and no async or API endpoint testing applies.

Full details: Test File Naming And Structure

Explanation

The pull request passes the test naming and structure check. The changed backend module is src/backend/tests/unit/components/files_and_knowledge/test_file_component.py, which follows the test_*.py pattern and uses pytest classes, fixtures, decorators, and assertions. The added tests have descriptive names for S3 and Google Drive download failures. They use tmp_path and scoped patches for setup and cleanup. The file includes negative download-failure tests and positive successful-download cleanup tests. No frontend test file or integration-test change is present. The existing integration-labeled section is unchanged by this pull request.

Full details: Excessive Mock Usage Warning

Explanation

PASS. The changed tests do not excessively mock core logic. Each test calls the real FileComponent._read_from_aws_s3() or _read_from_google_drive(), creates a real temporary file under tmp_path, and checks the real file path after cleanup. The mocks isolate external cloud clients and simulate the Windows open-file deletion failure through NamedTemporaryFile and Path.unlink, which directly tests the cleanup interaction. The existing cloud-client MagicMock usage remains focused on external dependencies.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}