Skip to content

fix(storage): close cloud download response bodies - #44419

Open
mikamikasuki wants to merge 1 commit into
langgenius:mainfrom
mikamikasuki:fix/boto-storage-stream-cleanup-44416
Open

mikamikasuki wants to merge 1 commit into
langgenius:mainfrom
mikamikasuki:fix/boto-storage-stream-cleanup-44416

Conversation

@mikamikasuki

@mikamikasuki mikamikasuki commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Close owned response bodies in eager and streaming download paths for AWS S3, Oracle OCI, Tencent COS, Alibaba OSS, Baidu BOS, Huawei OBS, and Volcengine TOS. Cleanup uses the existing close_stream helper in finally blocks, including early stream cancellation and read failures.

Fixes #44416

Regression coverage

Tests use real SDK response wrappers and in-memory HTTP streams; only client methods are replaced, so they make no cloud requests. The Aliyun OSS and Volcengine TOS streamed-read failure tests fail on upstream main at aa35579 because the underlying HTTP responses remain open after read() raises, and pass with this fix.

Validation

  • uv run --project api --dev pytest api/tests/unit_tests/extensions/storage/test_other_cloud_storage_streams.py api/tests/unit_tests/extensions/storage/test_s3_compatible_storage_streams.py api/tests/unit_tests/extensions/storage/test_tencent_cos_storage_streams.py api/tests/unit_tests/extensions/storage/test_aws_s3_storage.py -q — 26 passed.
  • make type-check PATH_TO_CHECK='api/extensions/storage/aliyun_oss_storage.py api/extensions/storage/baidu_obs_storage.py api/extensions/storage/huawei_obs_storage.py api/extensions/storage/volcengine_tos_storage.py api/extensions/storage/aws_s3_storage.py api/extensions/storage/oracle_oci_storage.py api/extensions/storage/tencent_cos_storage.py api/tests/unit_tests/extensions/storage/test_other_cloud_storage_streams.py api/tests/unit_tests/extensions/storage/test_s3_compatible_storage_streams.py api/tests/unit_tests/extensions/storage/test_tencent_cos_storage_streams.py' — passed.
  • Ruff check and format check on the changed test file — passed; the existing storage changes had also passed checks earlier.
  • uv run --project api python scripts/check_no_new_test_doubles.py --base-rev aa35579b66615a9620c6dfb0410b1d6cd99a4fea — passed.
  • git diff --check — passed.

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've verified the change and added or updated tests where meaningful regression risk justifies coverage.
  • I've updated the documentation accordingly.
  • I ran the full repository lint and type-check commands.

From Codex

@github-actions github-actions Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Oct 20, 2026
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-10-20 09:38:21.613005907 +0000
+++ /tmp/pyrefly_pr.txt	2026-10-20 09:38:13.373305788 +0000
@@ -4758,6 +4758,14 @@
   --> tests/unit_tests/extensions/logstore/repositories/test_logstore_api_workflow_node_execution_repository.py:27:64
 ERROR Object of class `Span` has no attribute `events` [missing-attribute]
    --> tests/unit_tests/extensions/otel/test_retrieval_tracing.py:165:42
+ERROR `(key: Unknown) -> GetObjectResult` is not assignable to attribute `get_object` with type `(key: Unknown, byte_range: Unknown | None = None, headers: Unknown | None = None, progress_callback: Unknown | None = None, process: Unknown | None = None, params: Unknown | None = None) -> GetObjectResult` [bad-assignment]
+   --> tests/unit_tests/extensions/storage/test_other_cloud_storage_streams.py:151:25
+ERROR Object of class `BceResponse` has no attribute `data` [missing-attribute]
+   --> tests/unit_tests/extensions/storage/test_other_cloud_storage_streams.py:158:5
+ERROR Object of class `HTTPResponse` has no attribute `request_id` [missing-attribute]
+   --> tests/unit_tests/extensions/storage/test_other_cloud_storage_streams.py:174:5
+ERROR `(bucket: str, key: str) -> GetObjectOutput` is not assignable to attribute `get_object` with type `(bucket: str, key: str, version_id: str = ..., if_match: str = ..., if_modified_since: datetime = ..., if_none_match: str = ..., if_unmodified_since: datetime = ..., ssec_algorithm: str = ..., ssec_key: str = ..., ssec_key_md5: str = ..., response_cache_control: str = ..., response_content_disposition: str = ..., response_content_encoding: str = ..., response_content_language: str = ..., response_content_type: str = ..., response_expires: datetime = ..., range_start: int = ..., range_end: int = ..., data_transfer_listener: Unknown | None = None, rate_limiter: Unknown | None = None, range: str = ..., traffic_limit: int = ..., process: str = ..., save_bucket: str = ..., save_object: str = ..., generic_input: GenericInput = ...) -> GetObjectOutput` [bad-assignment]
+   --> tests/unit_tests/extensions/storage/test_other_cloud_storage_streams.py:185:25
 ERROR Generator function should return `Generator` [bad-return]
   --> tests/unit_tests/extensions/test_workflow_warm_shutdown.py:13:36
 ERROR Argument `list[FromClause]` is not assignable to parameter `tables` with type `Sequence[Table] | None` in function `sqlalchemy.sql.schema.MetaData.create_all` [bad-argument-type]

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 76.40% 76.40% +0.01%
Strict coverage 76.08% 76.08% +0.01%
Typed symbols 64,953 64,977 +24
Untyped symbols 20,151 20,151 0
Modules 3856 3859 +3

@codecov

codecov Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.92%. Comparing base (9253852) to head (28a2f0b).

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #44419    +/-   ##
========================================
  Coverage   88.91%   88.92%            
========================================
  Files        5572     5573     +1     
  Lines      333914   334009    +95     
  Branches    66831    66833     +2     
========================================
+ Hits       296908   297011   +103     
+ Misses      31845    31839     -6     
+ Partials     5161     5159     -2     
Flag Coverage Δ
api 89.00% <98.63%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mikamikasuki mikamikasuki changed the title fix(storage): close S3 and OCI download streams fix(storage): close cloud download response bodies Oct 20, 2026
@github-actions github-actions Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Oct 20, 2026
@mikamikasuki
mikamikasuki force-pushed the fix/boto-storage-stream-cleanup-44416 branch 2 times, most recently from e8dd0c3 to f8ab34d Compare October 20, 2026 09:37
@github-actions github-actions Bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Oct 20, 2026
@mikamikasuki
mikamikasuki force-pushed the fix/boto-storage-stream-cleanup-44416 branch from d3b3c20 to cdcd25b Compare October 20, 2026 10:26
@mikamikasuki
mikamikasuki force-pushed the fix/boto-storage-stream-cleanup-44416 branch from 2ab56a5 to 28a2f0b Compare October 21, 2026 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cloud storage download paths leave response bodies open

1 participant