more error logs and return correct msg#1219
Conversation
|
/run-security-scan |
alexcos20
left a comment
There was a problem hiding this comment.
AI automated code review (Gemini 3).
Overall risk: low
Summary:
This pull request significantly improves error handling, logging, and client-facing error messages for the getStreamableLogs functionality. It introduces more informative error responses for 404 and 500 status codes in the command handler and the HTTP route. Additionally, robust error logging is added in the compute_engine_docker.ts component and the getStreamableLogs.ts command handler. Unit tests for the ComputeGetStreamableLogsHandler validation have also been added.
Comments:
• [INFO][other] Good addition of an error log here. It helps with debugging when the streamable logs command fails within the C2D engine, providing context about the specific job.
• [INFO][other] Adding a specific error message for 404 cases (Job not found or not running) is a good improvement for client clarity.
• [INFO][other] This is a robust way to handle unknown error types in catch blocks, ensuring that error.message is accessed safely and a fallback string conversion is used. Consistency with the error message in the returned status object is also good.
• [INFO][other] The logic to construct the response body for non-streamable responses is well-thought-out. It correctly prioritizes the specific error message from the command handler and provides sensible defaults for 404 and other errors. This significantly improves the API's usability for clients.
• [INFO][other] Good job adding unit tests for the ComputeGetStreamableLogsHandler validation. Covering cases with missing consumerAddress and jobId ensures the basic command validation is working as expected.
Fixes #767
Changes proposed in this PR: