Skip to content

Commit de19d4e

Browse files
committed
refactor errors
1 parent e43527d commit de19d4e

8 files changed

Lines changed: 35 additions & 65 deletions

File tree

src/sap_cloud_sdk/agentgateway/_lob.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,11 @@ def get_ias_client_id_lob() -> str:
122122
destination properties are returned — no auth token exchange is performed.
123123
124124
Returns:
125-
The IAS client ID string, or ``""`` if the destination is not found
126-
or the property is absent.
125+
The IAS client ID string, or ``""`` if the ``clientId`` property is absent.
127126
128127
Raises:
129128
EnvironmentError: If ``APPFND_CONHOS_LANDSCAPE`` is not set.
129+
AgentGatewaySDKError: If the IAS destination is not found.
130130
Any exception raised by the destination client.
131131
"""
132132
dest_name = _ias_dest_name()
@@ -137,10 +137,7 @@ def get_ias_client_id_lob() -> str:
137137
options=ConsumptionOptions(skip_token_retrieval=True),
138138
)
139139
if not dest:
140-
logger.warning(
141-
"IAS destination '%s' not found — clientId will be empty", dest_name
142-
)
143-
return ""
140+
raise AgentGatewaySDKError(f"IAS destination '{dest_name}' not found")
144141
return dest.properties.get("clientId", "")
145142

146143

src/sap_cloud_sdk/agentgateway/agw_client.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -309,11 +309,10 @@ def get_ias_client_id(self) -> str:
309309
and returns the ``clientId`` destination property.
310310
311311
Returns:
312-
The IAS client ID string, or ``""`` if it cannot be resolved.
312+
The IAS client ID string.
313313
314314
Raises:
315-
Nothing — failures are caught, logged at WARNING level,
316-
and an empty string is returned instead.
315+
AgentGatewaySDKError: If the IAS client ID cannot be resolved.
317316
"""
318317
try:
319318
credentials_path = detect_customer_agent_credentials()
@@ -326,11 +325,10 @@ def get_ias_client_id(self) -> str:
326325

327326
# LoB flow — read clientId from the IAS destination properties
328327
return get_ias_client_id_lob()
329-
except Exception:
330-
logger.warning(
331-
"Could not resolve IAS client ID from destination — clientId will be empty"
332-
)
333-
return ""
328+
except AgentGatewaySDKError:
329+
raise
330+
except Exception as e:
331+
raise AgentGatewaySDKError(f"Could not resolve IAS client ID: {e}") from e
334332

335333
@record_metrics(Module.AGENTGATEWAY, Operation.AGENTGATEWAY_LIST_MCP_TOOLS)
336334
async def list_mcp_tools(

src/sap_cloud_sdk/agentgateway/user-guide.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ Returns the IAS client ID. Automatically detects agent type:
229229
- **Customer agents**: reads `client_id` directly from the mounted credentials file.
230230
- **LoB agents**: fetches the IAS destination (`sap-managed-runtime-ias-{landscape}`) at provider subaccount level and returns the `clientId` destination property.
231231

232-
Returns `""` and logs a `WARNING` if the value cannot be resolved.
232+
Raises `AgentGatewaySDKError` if the value cannot be resolved.
233233

234234
```python
235235
agw_client = create_client(tenant_subdomain="my-tenant")

src/sap_cloud_sdk/destination/client.py

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from typing import Any, Dict, List, Optional, Callable, TypeVar
88

99
from sap_cloud_sdk.core.telemetry import Module, Operation, record_metrics
10+
from sap_cloud_sdk.core.secret_resolver import read_from_mount_and_fallback_to_env_var
1011
from sap_cloud_sdk.destination._http import DestinationHttp, API_V1, API_V2
1112
from sap_cloud_sdk.destination._models import (
1213
AccessStrategy,
@@ -684,17 +685,12 @@ def get_service_instance_id(self) -> str:
684685
module ``destination``, instance ``default``).
685686
686687
Returns:
687-
The instance ID string, or ``""`` if the secret cannot be resolved.
688+
The instance ID string.
688689
689690
Raises:
690-
Nothing — resolution failures are caught, logged at WARNING level,
691-
and an empty string is returned instead.
691+
DestinationOperationError: If the instance ID cannot be resolved from secrets.
692692
"""
693693
try:
694-
from sap_cloud_sdk.core.secret_resolver import (
695-
read_from_mount_and_fallback_to_env_var,
696-
)
697-
698694
config = _DestinationInstanceConfig()
699695
read_from_mount_and_fallback_to_env_var(
700696
base_volume_mount="/etc/secrets/appfnd",
@@ -704,11 +700,10 @@ def get_service_instance_id(self) -> str:
704700
target=config,
705701
)
706702
return config.instanceid
707-
except Exception:
708-
logger.warning(
709-
"Could not resolve destination instance ID from secrets — instanceId will be empty"
710-
)
711-
return ""
703+
except Exception as e:
704+
raise DestinationOperationError(
705+
"Could not resolve destination instance ID from secrets"
706+
) from e
712707

713708
# ---------- Internal helpers ----------
714709

src/sap_cloud_sdk/destination/user-guide.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -755,4 +755,4 @@ The function reads the `instanceid` field from the secret resolved via the stand
755755
- **Mount path**: `/etc/secrets/appfnd/destination/default/instanceid`
756756
- **Env var fallback**: `CLOUD_SDK_CFG_DESTINATION_DEFAULT_INSTANCEID`
757757

758-
Returns `""` and logs a `WARNING` if the secret cannot be resolved (e.g., running locally without a binding).
758+
Raises `DestinationOperationError` if the secret cannot be resolved (e.g., running locally without a binding).

tests/agentgateway/unit/test_agw_client.py

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,14 +1012,13 @@ def test_customer_returns_client_id_from_credentials(self):
10121012

10131013
assert result == "customer-client-id"
10141014

1015-
def test_customer_returns_empty_string_on_load_failure(self):
1015+
def test_customer_raises_on_load_failure(self):
10161016
with (
10171017
patch(_DETECT_CREDS_PATCH, return_value="/etc/ums/credentials/credentials"),
10181018
patch(_LOAD_CREDS_PATCH, side_effect=Exception("parse error")),
10191019
):
1020-
result = create_client().get_ias_client_id()
1021-
1022-
assert result == ""
1020+
with pytest.raises(AgentGatewaySDKError, match="Could not resolve IAS client ID"):
1021+
create_client().get_ias_client_id()
10231022

10241023
# --- LoB flow ---
10251024

@@ -1031,11 +1030,10 @@ def test_lob_returns_client_id_from_destination_properties(self, _mock_detect):
10311030
assert result == "lob-client-id"
10321031

10331032
@_NO_CUSTOMER_CREDS
1034-
def test_lob_returns_empty_string_when_destination_not_found(self, _mock_detect):
1035-
with patch(_GET_IAS_CLIENT_ID_LOB_PATCH, return_value=""):
1036-
result = create_client(tenant_subdomain="my-tenant").get_ias_client_id()
1037-
1038-
assert result == ""
1033+
def test_lob_raises_when_destination_not_found(self, _mock_detect):
1034+
with patch(_GET_IAS_CLIENT_ID_LOB_PATCH, side_effect=AgentGatewaySDKError("IAS destination 'sap-managed-runtime-ias-eu10' not found")):
1035+
with pytest.raises(AgentGatewaySDKError, match="IAS destination"):
1036+
create_client(tenant_subdomain="my-tenant").get_ias_client_id()
10391037

10401038
@_NO_CUSTOMER_CREDS
10411039
def test_lob_returns_empty_string_when_property_absent(self, _mock_detect):
@@ -1045,13 +1043,7 @@ def test_lob_returns_empty_string_when_property_absent(self, _mock_detect):
10451043
assert result == ""
10461044

10471045
@_NO_CUSTOMER_CREDS
1048-
def test_lob_returns_empty_string_and_logs_warning_on_exception(self, _mock_detect):
1049-
with (
1050-
patch(_GET_IAS_CLIENT_ID_LOB_PATCH, side_effect=EnvironmentError("APPFND_CONHOS_LANDSCAPE not set")),
1051-
patch("sap_cloud_sdk.agentgateway.agw_client.logger") as mock_logger,
1052-
):
1053-
result = create_client(tenant_subdomain="my-tenant").get_ias_client_id()
1054-
1055-
assert result == ""
1056-
mock_logger.warning.assert_called_once()
1057-
assert "clientId" in mock_logger.warning.call_args[0][0]
1046+
def test_lob_raises_on_exception(self, _mock_detect):
1047+
with patch(_GET_IAS_CLIENT_ID_LOB_PATCH, side_effect=EnvironmentError("APPFND_CONHOS_LANDSCAPE not set")):
1048+
with pytest.raises(AgentGatewaySDKError, match="Could not resolve IAS client ID"):
1049+
create_client(tenant_subdomain="my-tenant").get_ias_client_id()

tests/agentgateway/unit/test_lob.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,17 +1037,16 @@ def test_returns_client_id_from_destination_properties(self):
10371037
options=ConsumptionOptions(skip_token_retrieval=True),
10381038
)
10391039

1040-
def test_returns_empty_string_when_destination_not_found(self):
1040+
def test_raises_when_destination_not_found(self):
10411041
mock_dest_client = MagicMock()
10421042
mock_dest_client.get_destination.return_value = None
10431043

10441044
with (
10451045
patch("sap_cloud_sdk.agentgateway._lob._ias_dest_name", return_value="sap-managed-runtime-ias-eu10"),
10461046
patch("sap_cloud_sdk.agentgateway._lob.create_destination_client", return_value=mock_dest_client),
10471047
):
1048-
result = get_ias_client_id_lob()
1049-
1050-
assert result == ""
1048+
with pytest.raises(AgentGatewaySDKError, match="sap-managed-runtime-ias-eu10"):
1049+
get_ias_client_id_lob()
10511050

10521051
def test_returns_empty_string_when_property_absent(self):
10531052
mock_dest = MagicMock()

tests/destination/unit/test_client.py

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2222,7 +2222,7 @@ def test_patch_destination_labels_without_tenant_uses_provider_context(self):
22222222
assert kwargs["tenant_subdomain"] is None
22232223

22242224

2225-
_RESOLVER_PATCH = "sap_cloud_sdk.core.secret_resolver.read_from_mount_and_fallback_to_env_var"
2225+
_RESOLVER_PATCH = "sap_cloud_sdk.destination.client.read_from_mount_and_fallback_to_env_var"
22262226

22272227

22282228
class TestGetServiceInstanceId:
@@ -2248,19 +2248,8 @@ def fill_instanceid(*args, **kwargs):
22482248
)
22492249

22502250
@patch(_RESOLVER_PATCH, side_effect=RuntimeError("mount failed"))
2251-
def test_returns_empty_string_on_exception(self, _mock_read):
2251+
def test_raises_on_exception(self, _mock_read):
22522252
client = DestinationClient(MagicMock())
22532253

2254-
result = client.get_service_instance_id()
2255-
2256-
assert result == ""
2257-
2258-
@patch(_RESOLVER_PATCH, side_effect=RuntimeError("mount failed"))
2259-
def test_logs_warning_on_exception(self, _mock_read):
2260-
client = DestinationClient(MagicMock())
2261-
2262-
with patch("sap_cloud_sdk.destination.client.logger") as mock_logger:
2254+
with pytest.raises(DestinationOperationError, match="Could not resolve destination instance ID from secrets"):
22632255
client.get_service_instance_id()
2264-
2265-
mock_logger.warning.assert_called_once()
2266-
assert "instanceId" in mock_logger.warning.call_args[0][0]

0 commit comments

Comments
 (0)