From c0813d05fd57634dc7be12c569a27d8b56da917f Mon Sep 17 00:00:00 2001
From: mocha06 <52426811+mocha06@users.noreply.github.com>
Date: Sat, 18 Jul 2026 01:17:11 -0300
Subject: [PATCH] fix(sdk): tag raising S3 PUT failures with the s3_upload step

AttachmentService promised every step surfaces as AttachmentUploadError
carrying step, but the S3 put call was not wrapped: an httpx transport error
or the uploader's host-allowlist ValueError bubbled bare, so the step-aware
envelope degraded to a generic untagged error for exactly the network /
tampered-URL failures where the stage tag matters most. The put call is now
wrapped and re-raised as AttachmentUploadError(step="s3_upload") from the
cause, keeping the attachment and knowledge-base document upload pipelines on
one contract. Existing HTTP >= 400 tagging is unchanged.

Closes #430
---
 .../pipefy_sdk/services/attachment_service.py | 17 ++++++++++-----
 .../tests/services/test_attachment_service.py | 21 +++++++++++++++++++
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/packages/sdk/src/pipefy_sdk/services/attachment_service.py b/packages/sdk/src/pipefy_sdk/services/attachment_service.py
index 3c28d456..b293ec90 100644
--- a/packages/sdk/src/pipefy_sdk/services/attachment_service.py
+++ b/packages/sdk/src/pipefy_sdk/services/attachment_service.py
@@ -164,11 +164,18 @@ async def upload_attachment(
             )
         upload_url = upload_url.strip()
 
-        put_result = await self._s3_uploader.put(
-            url=upload_url,
-            bytes_=file.bytes,
-            content_type=effective_type,
-        )
+        try:
+            put_result = await self._s3_uploader.put(
+                url=upload_url,
+                bytes_=file.bytes,
+                content_type=effective_type,
+            )
+        except Exception as exc:
+            # Transport errors and the uploader's host-allowlist rejection are
+            # s3_upload-stage failures; tag them so the step contract holds.
+            raise AttachmentUploadError(
+                f"S3 upload failed: {exc}", step="s3_upload"
+            ) from exc
         status = put_result.get("status_code", 0)
         if not isinstance(status, int) or status >= 400:
             body_snippet = put_result.get("body_snippet")
diff --git a/packages/sdk/tests/services/test_attachment_service.py b/packages/sdk/tests/services/test_attachment_service.py
index 9f7aa539..2295d71f 100644
--- a/packages/sdk/tests/services/test_attachment_service.py
+++ b/packages/sdk/tests/services/test_attachment_service.py
@@ -429,6 +429,27 @@ async def test_upload_attachment_s3_http_error_carries_snippet_and_status(tmp_pa
     assert ctx.value.status_code == 403
 
 
+@pytest.mark.unit
+@pytest.mark.asyncio
+async def test_upload_attachment_s3_put_exception_tagged_s3_upload(tmp_path):
+    """A raising PUT (transport error, allowlist rejection) carries the step tag."""
+    service, _ = _make_service()
+    service._s3_uploader.put = AsyncMock(
+        side_effect=ConnectionError("connection reset by peer")
+    )
+    attachment = _build_attachment(tmp_path, name="a.bin")
+
+    with pytest.raises(AttachmentUploadError) as ctx:
+        await service.upload_attachment(
+            attachment,
+            organization_id="org-1",
+            target=CardTarget(card_id="c1", field_id="f"),
+        )
+    assert ctx.value.step == "s3_upload"
+    assert "connection reset by peer" in str(ctx.value)
+    assert isinstance(ctx.value.__cause__, ConnectionError)
+
+
 @pytest.mark.unit
 @pytest.mark.asyncio
 async def test_upload_attachment_extract_storage_path_failure_maps_to_s3_step(tmp_path):