Skip to content

chore(kagent-adk/session_service): add logging for Kagent API errors #1029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore(kagent-adk/session_service): add logging for Kagent API errors #1029

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 21 additions & 4 deletions python/packages/kagent-adk/src/kagent/adk/_session_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ async def create_session(
json=request_data,
headers={"X-User-ID": user_id},
)
response.raise_for_status()
try:
response.raise_for_status()
except httpx.HTTPStatusError:
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

The error handling pattern is duplicated across multiple methods. Consider extracting this into a helper method or decorator to reduce code duplication and improve maintainability.

Copilot uses AI. Check for mistakes.
logger.error(f"Failed to create session: {response.text}")
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Logging the full response text could potentially expose sensitive information. Consider logging only the status code and reason phrase, or sanitize the response content before logging.

Suggested change
logger.error(f"Failed to create session: {response.text}")
logger.error(
f"Failed to create session: status_code={response.status_code}, reason={response.reason_phrase}"
)

Copilot uses AI. Check for mistakes.
raise

data = response.json()
if not data.get("data"):
Expand Down Expand Up @@ -116,13 +120,18 @@ async def get_session(
except httpx.HTTPStatusError as e:
if e.response.status_code == 404:
return None
logger.error(f"Failed to get session: {response.text}")
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Logging the full response text could potentially expose sensitive information. Consider logging only the status code and reason phrase, or sanitize the response content before logging.

Copilot uses AI. Check for mistakes.
raise

@override
async def list_sessions(self, *, app_name: str, user_id: str) -> ListSessionsResponse:
# Make API call to list sessions
response = await self.client.get(f"/api/sessions?user_id={user_id}", headers={"X-User-ID": user_id})
response.raise_for_status()
try:
response.raise_for_status()
except httpx.HTTPStatusError:
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

The error handling pattern is duplicated across multiple methods. Consider extracting this into a helper method or decorator to reduce code duplication and improve maintainability.

Copilot uses AI. Check for mistakes.
logger.error(f"Failed to list sessions: {response.text}")
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Logging the full response text could potentially expose sensitive information. Consider logging only the status code and reason phrase, or sanitize the response content before logging.

Suggested change
logger.error(f"Failed to list sessions: {response.text}")
logger.error(f"Failed to list sessions: status_code={response.status_code}, reason={getattr(response, 'reason_phrase', '')}")

Copilot uses AI. Check for mistakes.
raise

data = response.json()
sessions_data = data.get("data", [])
Expand All @@ -145,7 +154,11 @@ async def delete_session(self, *, app_name: str, user_id: str, session_id: str)
f"/api/sessions/{session_id}?user_id={user_id}",
headers={"X-User-ID": user_id},
)
response.raise_for_status()
try:
response.raise_for_status()
except httpx.HTTPStatusError:
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

The error handling pattern is duplicated across multiple methods. Consider extracting this into a helper method or decorator to reduce code duplication and improve maintainability.

Copilot uses AI. Check for mistakes.
logger.error(f"Failed to delete session: {response.text}")
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Logging the full response text could potentially expose sensitive information. Consider logging only the status code and reason phrase, or sanitize the response content before logging.

Copilot uses AI. Check for mistakes.
raise

@override
async def append_event(self, session: Session, event: Event) -> Event:
Expand All @@ -161,7 +174,11 @@ async def append_event(self, session: Session, event: Event) -> Event:
json=event_data,
headers={"X-User-ID": session.user_id},
)
response.raise_for_status()
try:
response.raise_for_status()
except httpx.HTTPStatusError:
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

The error handling pattern is duplicated across multiple methods. Consider extracting this into a helper method or decorator to reduce code duplication and improve maintainability.

Copilot uses AI. Check for mistakes.
logger.error(f"Failed to append event: {response.text}")
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Logging the full response text could potentially expose sensitive information. Consider logging only the status code and reason phrase, or sanitize the response content before logging.

Copilot uses AI. Check for mistakes.
raise

# TODO: potentially pull and update the session from the server
# Update the in-memory session.
Expand Down