fix(files): raise CogniteFileUploadError instead of leaking CogniteHTTPStatusError on blob PUT failures (#2646)

Author: haakonvt

cognite/client/_api/files.py

@@ -36,7 +36,12 @@
     TimestampRange,
 )
 from cognite.client.data_classes.data_modeling import NodeId
-from cognite.client.exceptions import CogniteAPIError, CogniteAuthorizationError, CogniteFileUploadError
+from cognite.client.exceptions import (
+    CogniteAPIError,
+    CogniteAuthorizationError,
+    CogniteFileUploadError,
+    CogniteHTTPStatusError,
+)
 from cognite.client.utils._auxiliary import append_url_path, find_duplicates, unpack_items
 from cognite.client.utils._concurrency import AsyncSDKTask, execute_async_tasks
 from cognite.client.utils._identifier import Identifier, IdentifierSequence
@@ -783,16 +788,17 @@ async def _upload_bytes(
         if file_size is not None:
             headers["Content-Length"] = str(file_size)
 
-        upload_response = await self._request(
-            "PUT",
-            full_url=full_upload_url,
-            content=file_content,
-            headers=headers,
-            timeout=self._config.file_transfer_timeout,
-            semaphore=self._get_semaphore("upload"),
-        )
-        if not upload_response.is_success:
-            raise CogniteFileUploadError(message=upload_response.text, code=upload_response.status_code)
+        try:
+            await self._request(
+                "PUT",
+                full_url=full_upload_url,
+                content=file_content,
+                headers=headers,
+                timeout=self._config.file_transfer_timeout,
+                semaphore=self._get_semaphore("upload"),
+            )
+        except CogniteHTTPStatusError as err:
+            raise CogniteFileUploadError(message=err.response.text, code=err.status_code) from None
         return file_metadata
 
     async def upload_bytes(
@@ -1074,16 +1080,17 @@ async def _upload_multipart_part(
         if file_size is not None:
             headers["Content-Length"] = str(file_size)
 
-        upload_response = await self._request(
-            "PUT",
-            full_url=upload_url,
-            content=file_content,
-            headers=headers,
-            timeout=self._config.file_transfer_timeout,
-            semaphore=self._get_semaphore("upload"),
-        )
-        if not upload_response.is_success:
-            raise CogniteFileUploadError(message=upload_response.text, code=upload_response.status_code)
+        try:
+            await self._request(
+                "PUT",
+                full_url=upload_url,
+                content=file_content,
+                headers=headers,
+                timeout=self._config.file_transfer_timeout,
+                semaphore=self._get_semaphore("upload"),
+            )
+        except CogniteHTTPStatusError as err:
+            raise CogniteFileUploadError(message=err.response.text, code=err.status_code) from None
 
     async def _complete_multipart_upload(self, session: FileMultipartUploadSession) -> None:
         """Complete a multipart upload. Once this returns the file can be downloaded.

cognite/client/_sync_api/files.py

@@ -1,6 +1,6 @@
 """
 ===============================================================================
-d8fa16955b8c30a8af32a19dcb33c4a8
+44f1fb8ed3bbf859ea01358b27381995
 This file is auto-generated from the Async API modules, - do not edit manually!
 ===============================================================================
 """

tests/tests_unit/test_api/test_files.py

@@ -29,7 +29,7 @@
     FileMetadataWrite,
 )
 from cognite.client.data_classes.labels import Label, LabelFilter
-from cognite.client.exceptions import CogniteAPIError, CogniteAuthorizationError
+from cognite.client.exceptions import CogniteAPIError, CogniteAuthorizationError, CogniteFileUploadError
 from tests.tests_unit.conftest import DefaultResourceGenerator
 from tests.utils import get_or_raise, get_url, jsgz_load
 
@@ -836,6 +836,57 @@ def test_upload_path_does_not_exist(self, cognite_client: CogniteClient) -> None
         with pytest.raises(FileNotFoundError):
             cognite_client.files.upload(path=Path("/no/such/path"))
 
+    def test_upload_bytes_400_raises_file_upload_error(
+        self,
+        cognite_client: CogniteClient,
+        example_file: dict[str, Any],
+        async_client: AsyncCogniteClient,
+        httpx_mock: HTTPXMock,
+    ) -> None:
+        """Bug in 8.0.0 to 8.6.0: a 400 from the blob storage PUT raised CogniteHTTPStatusError instead of CogniteFileUploadError."""
+        httpx_mock.add_response(
+            method="POST",
+            url=get_url(async_client.files) + "/files?overwrite=false",
+            status_code=200,
+            json=example_file,
+        )
+        httpx_mock.add_response(method="PUT", url="https://upload.here", status_code=400, text="Bad Request")
+
+        with pytest.raises(CogniteFileUploadError) as exc_info:
+            cognite_client.files.upload_bytes(content=b"content", name="bla")
+
+        assert exc_info.value.code == 400
+
+    def test_upload_content_part_400_raises_file_upload_error(
+        self,
+        cognite_client: CogniteClient,
+        example_file: dict[str, Any],
+        async_client: AsyncCogniteClient,
+        httpx_mock: HTTPXMock,
+        tmp_path: Path,
+    ) -> None:
+        """Bug in 8.0.0 to 8.6.0: a 400 from the blob storage PUT during multipart upload raised CogniteHTTPStatusError
+        instead of CogniteFileUploadError.
+        """
+        multipart_response = {
+            "items": [{**example_file, "uploadUrls": ["https://upload.here/part0"], "uploadId": "test-id"}]
+        }
+        httpx_mock.add_response(
+            method="POST",
+            url=re.compile(re.escape(get_url(async_client.files) + "/files/multiuploadlink") + r"\?.*"),
+            status_code=200,
+            json=multipart_response,
+        )
+        httpx_mock.add_response(method="PUT", url="https://upload.here/part0", status_code=400, text="Bad Request")
+
+        test_file = tmp_path / "test.bin"
+        test_file.write_bytes(b"x")
+
+        with pytest.raises(CogniteFileUploadError) as exc_info:
+            cognite_client.files.upload_content(path=test_file, instance_id=NodeId("test-space", "test-0001"))
+
+        assert exc_info.value.code == 400
+
     @pytest.mark.parametrize(
         "file_size, expected_parts",
         [