-
Notifications
You must be signed in to change notification settings - Fork 5
fix: proper error handling #114
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors error handling in the session and runtime modules to leverage Python 3.11 exception groups, tightens shutdown logic, and updates package metadata.
- Use
asyncio.wait_forwith a timeout when cancelling the session consumer task and update exception catches to handleasyncio.TimeoutError - Consolidate exception logging to use
except* Exceptionand add specific handling forHTTPStatusError - Bump package version to 0.0.93, require Python >=3.11, and update dependency versions
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/uipath_mcp/_cli/_runtime/_session.py | Refactor task cancellation logic and switch to exception‐group logging in the server loop |
| src/uipath_mcp/_cli/_runtime/_runtime.py | Replace broad BaseException catch with except* Exception, simplify traceback logging, handle HTTP errors |
| pyproject.toml | Bump version to 0.0.93, require Python >=3.11, and update downstream dependency versions |
| .python-version | Update local Python version from 3.10 to 3.11 |
Comments suppressed due to low confidence (1)
src/uipath_mcp/_cli/_runtime/_session.py:153
- [nitpick] The updated message is less descriptive than the original ‘Error in server process…’; consider restoring context about where the error occurred to aid debugging.
f"Unexpected error for session {self._session_id}: {e}",
| except (asyncio.CancelledError, asyncio.TimeoutError): | ||
| pass |
Copilot
AI
Jul 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Swallowing TimeoutError silently may hide issues if the consumer task doesn’t shut down as expected; consider logging a warning or taking remedial action on timeout.
| except (asyncio.CancelledError, asyncio.TimeoutError): | |
| pass | |
| except asyncio.CancelledError: | |
| pass | |
| except asyncio.TimeoutError: | |
| logger.warning( | |
| f"Timeout occurred while shutting down consumer task for session {self._session_id}. Task may not have terminated cleanly." | |
| ) |
| except Exception as e: | ||
| logger.error(f"Error during registration: {e}") | ||
| if e.status_code == 400: | ||
| logger.error(f"Error details: {e.response.text}") | ||
| if isinstance(e, HTTPStatusError): | ||
| logger.error( | ||
| f"HTTP error details: {e.response.text} status code: {e.response.status_code}" | ||
| ) | ||
|
|
Copilot
AI
Jul 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider catching HTTPStatusError explicitly before a general Exception to differentiate HTTP errors from other failures and avoid large isinstance checks in the generic handler.
| if isinstance(e, HTTPStatusError): | ||
| logger.error( | ||
| f"HTTP error details: {e.response.text} status code: {e.response.status_code}" | ||
| ) |
Copilot
AI
Jul 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Removing this info-level log may reduce visibility into client configuration during troubleshooting; consider demoting it to logger.debug instead of dropping it entirely.
Summary
This PR refactors error handling in the session and runtime modules to leverage Python 3.11 exception groups, tightens shutdown logic, and updates package metadata.
asyncio.wait_forwith a timeout when cancelling the session consumer task and update exception catches to handleasyncio.TimeoutErrorexcept* Exceptionand add specific handling forHTTPStatusErrorDevelopment Package