feat: only valid smartsheet responses get cached (#14)

* feat: only valid smartsheet responses get cached

Also adds new pytest for checking that invalid smartsheet responses don't get cached

* feat: updates get_smartsheet method to check the Smartsheet SDK response for errors

Also updates test_get_smartsheet_fail logic and adds new test for raising HTTP exceptions for smartsheet API errors

* test: removes unused import

* feat: edits get_smartsheet to raise Exception on instance of Error model

* style: removes spacing

* test: update get_smartsheet tests to match new error handling

- Added SmartsheetError mocking
- Adjusted generic exception test
- Restored to_json assertions in successful case

Author: johendrickson

aind-smartsheet-service-server/src/aind_smartsheet_service_server/route.py

@@ -3,9 +3,10 @@
 from asyncio import to_thread
 from typing import List, Optional
 
-from fastapi import APIRouter, Query, status
+from fastapi import APIRouter, HTTPException, Query, status
 from fastapi_cache.decorator import cache
 from smartsheet import Smartsheet
+from smartsheet.models.error import Error as SmartsheetError
 
 from aind_smartsheet_service_server.configs import settings
 from aind_smartsheet_service_server.handler import SheetHandler
@@ -29,8 +30,7 @@ async def get_smartsheet(sheet_id: int) -> str:
 
     Returns
     -------
-    str
-
+    str or raises Exception
     """
 
     client = Smartsheet(
@@ -39,6 +39,12 @@ async def get_smartsheet(sheet_id: int) -> str:
         access_token=(settings.access_token.get_secret_value()),
     )
     sheet = await to_thread(client.Sheets.get_sheet, sheet_id)
+
+    if isinstance(sheet, SmartsheetError):
+        status = sheet.result.status_code
+        message = sheet.result.message or "Smartsheet error"
+        raise HTTPException(status_code=status, detail=message)
+
     return sheet.to_json()
 
 

aind-smartsheet-service-server/tests/test_route.py

@@ -3,6 +3,8 @@
 from unittest.mock import AsyncMock, MagicMock, call, patch
 
 import pytest
+from fastapi import HTTPException
+from smartsheet.models.error import Error as SmartsheetError
 from starlette.testclient import TestClient
 
 from aind_smartsheet_service_server.route import get_smartsheet
@@ -28,6 +30,28 @@ async def test_get_smartsheet(self, mock_get_sheet: MagicMock):
         mock_get_sheet.assert_has_calls([call(0), call().to_json()])
         assert '{"a": "b"}' == sheet
 
+    @patch("smartsheet.sheets.Sheets.get_sheet")
+    async def test_get_smartsheet_fail(self, mock_get_sheet: MagicMock):
+        """Tests generic exception handling"""
+        mock_get_sheet.side_effect = Exception("Fail")
+        with pytest.raises(Exception) as e:
+            _ = await get_smartsheet(sheet_id=0)
+        assert "Fail" in str(e.value)
+
+    @patch("smartsheet.sheets.Sheets.get_sheet")
+    async def test_get_smartsheet_error(self, mock_get_sheet: MagicMock):
+        """Tests SmartsheetError triggers HTTPException"""
+        error_obj = SmartsheetError(MagicMock())
+        error_obj.result = MagicMock()
+        error_obj.result.status_code = 404
+        error_obj.result.message = "Not Found"
+        mock_get_sheet.return_value = error_obj
+
+        with pytest.raises(HTTPException) as e:
+            _ = await get_smartsheet(sheet_id=0)
+        assert e.value.status_code == 404
+        assert "Not Found" in str(e.value.detail)
+
     @patch("aind_smartsheet_service_server.route.get_smartsheet")
     async def test_get_funding(
         self,