Skip to content

fix: Distinguish request failures from engine errors when result.json…#95

Open
LuckyYC wants to merge 4 commits into
mainfrom
dev
Open

fix: Distinguish request failures from engine errors when result.json…#95
LuckyYC wants to merge 4 commits into
mainfrom
dev

Conversation

@LuckyYC
Copy link
Copy Markdown
Collaborator

@LuckyYC LuckyYC commented Jun 4, 2026

fix: Distinguish request failures from engine errors when result.json is missing

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates st_engine/engine/llm_runner.py to distinguish between an engine crash and a request-level failure when the Locust result file is missing. It checks if the run-time limit was reached and if the process exited with a return code of 1 to set the status to FAILED_REQUESTS. Feedback suggests making this check more robust by checking both stdout and stderr for the completion message, guarding against None values to prevent potential TypeErrors, and ensuring process is not None before checking its return code.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread st_engine/engine/llm_runner.py Outdated
Comment on lines +861 to +864
run_time_completed = "--run-time limit reached" in stderr
locust_request_failure_exit = (
process.returncode is not None and process.returncode == 1
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Checking only stderr for the "--run-time limit reached" message can be fragile if Locust logs are redirected to stdout (e.g., via custom log configurations or container environments). Additionally, if stderr or stdout is None (such as in certain testing environments), performing an in check will raise a TypeError.\n\nTo make this check more robust and defensive, we should:\n1. Check both stdout and stderr for the completion message.\n2. Guard against None values for stdout and stderr.\n3. Simplify the process.returncode check while guarding against process being None.

                run_time_completed = (\n                    (stderr and "--run-time limit reached" in stderr) or\n                    (stdout and "--run-time limit reached" in stdout)\n                )\n                locust_request_failure_exit = (\n                    process is not None and process.returncode == 1\n                )

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.

1 participant