Foundation Model Serve Health Check #4638
Conversation
Test Results for assets-test0 tests 0 ✅ 0s ⏱️ Results for commit 1c6ce13. ♻️ This comment has been updated with latest results. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| except Exception as e: | ||
| logger.error(f"Downstream engine health check failed: {str(e)}") | ||
| return JSONResponse( | ||
| content={"status": "unhealthy", "downstream": "error", "error": str(e)}, |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 17 hours ago
To fix this problem, we should avoid sending the exception's string representation (str(e)) in responses returned to the user. Instead, log the exception on the server using the configured logger and return a generic error message to the client, e.g., “internal error.” Only minimal, non-sensitive information should be exposed in API responses. This change can be accomplished by editing the except Exception as e: block in the /health endpoint. The logger.error() call can still record the full details of the exception for server-side troubleshooting. No new methods or imports are needed, as appropriate logging is already in use.
Specifically:
- Remove the
"error": str(e)entry from the JSON response returned to the client on line 400. - Continue logging the error server-side as is.
Only assets/training/model_management/environments/foundation-model-serve/context/foundation/model/serve/api_server.py needs to be edited, within lines 397-401.
| @@ -397,7 +397,7 @@ | ||
| except Exception as e: | ||
| logger.error(f"Downstream engine health check failed: {str(e)}") | ||
| return JSONResponse( | ||
| content={"status": "unhealthy", "downstream": "error", "error": str(e)}, | ||
| content={"status": "unhealthy", "downstream": "error"}, | ||
| status_code=503 | ||
| ) | ||
|
|
..._management/environments/foundation-model-serve/context/foundation/model/serve/api_server.py
Fixed
Show fixed
Hide fixed
Test Results for training-model-mgmt-unittests45 tests 45 ✅ 11s ⏱️ Results for commit 1c6ce13. ♻️ This comment has been updated with latest results. |
| 503 Service Unavailable if downstream is not ready. | ||
| """ | ||
| try: | ||
| ready_path = os.getenv(EnvironmentVariables.CONTAINER_READY_CHECK_PATH, "/ready") |
There was a problem hiding this comment.
It will be better to abstract this code more reusable because readiness probe and heacth check seems to be similar.
..._management/environments/foundation-model-serve/context/foundation/model/serve/api_server.py
Outdated
Show resolved
Hide resolved
Test Results for scripts-test79 tests 79 ✅ 8m 48s ⏱️ Results for commit 1c6ce13. ♻️ This comment has been updated with latest results. |
..._management/environments/foundation-model-serve/context/foundation/model/serve/api_server.py
Show resolved
Hide resolved
| except Exception as e: | ||
| logger.error(f"Downstream engine readiness check failed: {str(e)}") | ||
| return JSONResponse( | ||
| content={"status": "not_ready", "downstream": "error", "error": str(e)}, |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 17 hours ago
To resolve this issue, the exception handling in the ready endpoint should avoid returning the raw error message (str(e)) to the client. Instead, log the exception and return a generic error response. This ensures that sensitive internal details are kept only in server logs, not exposed via the API response. Specifically, for lines 449–451, change the code so that logger.error logs the full exception and stack trace for developer use, but the JSON returned to the client only includes a fixed, non-detailed "internal error" message. Required edits include:
- Use
logger.errorwithexc_info=Trueto capture stack trace. - Remove the
"error": str(e)field from the response; replace with a generic"error": "internal_error"or similar.
No additional imports or methods are needed beyond what is already present.
| @@ -446,9 +446,9 @@ | ||
| status_code=503 | ||
| ) | ||
| except Exception as e: | ||
| logger.error(f"Downstream engine readiness check failed: {str(e)}") | ||
| logger.error(f"Downstream engine readiness check failed: {str(e)}", exc_info=True) | ||
| return JSONResponse( | ||
| content={"status": "not_ready", "downstream": "error", "error": str(e)}, | ||
| content={"status": "not_ready", "downstream": "error", "error": "internal_error"}, | ||
| status_code=503 | ||
| ) | ||
|
|
Foundation Model Serve Health Check