Skip to content

Conversation

@lpi-tn
Copy link
Contributor

@lpi-tn lpi-tn commented Jan 5, 2026

This pull request improves how API endpoint usage is logged and monitored. The main change is that the logging of endpoint requests is now handled asynchronously in the background, which should improve response times and reliability. Additionally, error handling and logging have been enhanced to ensure failures in logging do not affect the main application flow.

Middleware improvements:

  • Updated MonitorRequestsMiddleware in monitor_requests.py to use BackgroundTask for registering endpoint usage asynchronously, instead of blocking the response with a synchronous call. This allows the API to return responses faster and more reliably. [1] [2]

Logging and error handling:

  • Added improved error logging in sql_service.py for failures during endpoint registration, ensuring that issues are captured without impacting the user experience. [1] [2]

Copy link
Contributor

Copilot AI left a 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 pull request refactors the endpoint monitoring system to use asynchronous background tasks instead of blocking thread pool execution. The change improves API response times by deferring endpoint usage logging until after the response is sent to the client.

Key changes:

  • Replaced synchronous run_in_threadpool execution with Starlette's BackgroundTask for non-blocking endpoint registration
  • Added comprehensive error handling and logging for endpoint registration failures
  • Moved response generation to occur before endpoint logging setup

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/app/middleware/monitor_requests.py Refactored middleware to use BackgroundTask and moved response generation before logging setup
src/app/services/sql_service.py Added try-except block and error logging to register_endpoint method

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

session_id=session_id,
http_code=200,
response.background = BackgroundTask(
register_endpoint, request.url.path, session_id, 200
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The HTTP status code is hardcoded to 200, but response.status_code should be used instead to accurately log the actual response status (e.g., 404, 500). Replace 200 with response.status_code.

Suggested change
register_endpoint, request.url.path, session_id, 200
register_endpoint, request.url.path, session_id, response.status_code

Copilot uses AI. Check for mistakes.
Comment on lines 22 to 23
except Exception as e:
logger.error(f"Failed to register endpoint {request.url.path}: {e}")
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

This exception handler will never catch exceptions from register_endpoint because BackgroundTask executes after the response is returned. The try-except block should be removed from the middleware since error handling is now in sql_service.py's register_endpoint method.

Copilot uses AI. Check for mistakes.
Base automatically changed from cache_model to main January 6, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants