Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions application_sdk/services/objectstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ async def get_content(
logger.debug(f"Successfully retrieved file content: {key}")
return response_data

except FileNotFoundError:
raise
except Exception as e:
logger.error(f"Error getting file content for {key}: {str(e)}")
raise e
Expand Down Expand Up @@ -369,8 +371,6 @@ async def download_file(
... destination="/tmp/downloaded_report.pdf"
... )
"""
# Ensure directory exists

if not os.path.exists(os.path.dirname(destination)):
os.makedirs(os.path.dirname(destination), exist_ok=True)

Expand All @@ -381,6 +381,9 @@ async def download_file(
f.write(response_data)

logger.info(f"Successfully downloaded file: {source}")
except FileNotFoundError:
logger.debug(f"File not found in object store: {source}")
raise
except Exception as e:
logger.warning(
f"Failed to download file {source} from object store: {str(e)}"
Expand Down Expand Up @@ -451,8 +454,16 @@ async def _invoke_dapr_binding(
)
return response.data
except Exception as e:
logger.error(f"Error in Dapr binding operation '{operation}': {str(e)}")
raise
if "file not found" in str(e).lower() or "not found" in str(e).lower():
Copy link
Preview

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

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

The string matching logic is too broad and could incorrectly classify non-file-related "not found" errors. Consider using more specific error detection, such as checking exception types or more precise error message patterns that are specific to file operations.

Copilot uses AI. Check for mistakes.

logger.debug(
f"File not found in Dapr binding operation '{operation}' - this is normal for new workflows"
)
raise FileNotFoundError(
f"File not found: {metadata.get('key', 'unknown')}"
)
else:
logger.error(f"Error in Dapr binding operation '{operation}': {str(e)}")
raise

@classmethod
def _cleanup_local_path(cls, path: str) -> None:
Expand Down
38 changes: 38 additions & 0 deletions tests/unit/services/test_objectstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,44 @@ async def test_delete_prefix_list_failure(self, mock_list_files: AsyncMock) -> N
with pytest.raises(Exception, match="Failed to list files"):
await ObjectStore.delete_prefix(prefix="prefix/")

@patch("application_sdk.services.objectstore.DaprClient")
@patch("application_sdk.services.objectstore.logger")
async def test_get_content_file_not_found_logs_debug(
self, mock_logger: MagicMock, mock_dapr_client: MagicMock
) -> None:
"""Test that file not found errors are logged at DEBUG level."""
mock_client = MagicMock()
mock_client.invoke_binding.side_effect = Exception("file not found")
mock_dapr_client.return_value.__enter__.return_value = mock_client

with pytest.raises(Exception):
await ObjectStore.get_content("nonexistent/file.txt")

# Verify debug log was called instead of error log
mock_logger.debug.assert_called_once_with(
"File not found in object store: nonexistent/file.txt"
)
Copy link

Choose a reason for hiding this comment

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

Bug: Incorrect Debug Log in File Not Found Test

The test_get_content_file_not_found_logs_debug test asserts for an incorrect debug log message. When get_content encounters a file not found error, the _invoke_dapr_binding method logs "File not found in Dapr binding operation 'get' - this is normal for new workflows", not the "File not found in object store..." message the test expects. get_content itself only re-raises the exception.

Fix in Cursor Fix in Web

Copy link
Author

Choose a reason for hiding this comment

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

I am not sure about tests code would love any input

mock_logger.error.assert_not_called()

@patch("application_sdk.services.objectstore.DaprClient")
@patch("application_sdk.services.objectstore.logger")
async def test_get_content_other_error_logs_error(
self, mock_logger: MagicMock, mock_dapr_client: MagicMock
) -> None:
"""Test that non-file-not-found errors are still logged at ERROR level."""
mock_client = MagicMock()
mock_client.invoke_binding.side_effect = Exception("Connection timeout")
mock_dapr_client.return_value.__enter__.return_value = mock_client

with pytest.raises(Exception):
await ObjectStore.get_content("test/file.txt")

# Verify error log was called for non-file-not-found errors
mock_logger.error.assert_called_once_with(
"Error getting file content for test/file.txt: Connection timeout"
)
mock_logger.debug.assert_not_called()

# @patch("application_sdk.services.objectstore.ObjectStore.list_files", new_callable=AsyncMock)
# @patch("application_sdk.services.objectstore.ObjectStore._download_file", new_callable=AsyncMock)
# async def test_download_directory_success(
Expand Down