FIX: Include the exception type when a scenario fails to start - #2521
Open
varunj-msft wants to merge 1 commit into
Open
FIX: Include the exception type when a scenario fails to start#2521varunj-msft wants to merge 1 commit into
varunj-msft wants to merge 1 commit into
Conversation
varunj-msft
force-pushed
the
varunj-msft/v1.1.0-Release-Start-Error-Message
branch
from
September 1, 2026 18:04
c1d4b40 to
b9f5223
Compare
varunj-msft
force-pushed
the
varunj-msft/v1.1.0-Release-Start-Error-Message
branch
4 times, most recently
from
September 1, 2026 22:22
21ab94d to
7547ef9
Compare
httpx.ReadTimeout carries no message, so str(exc) is the empty string and the scenario start failure printed "Error starting scenario: " with nothing after it. That is the exact failure the end-to-end tests hit when the server takes longer than the client read timeout to start a run, and it gave no indication of what went wrong. Route both start-run failure paths through _print_cli_exception, which the CLI already uses elsewhere. It reports the exception class and falls back to repr() when str() is empty. Handle the timeout at the entry points rather than teaching the shared printer who its caller is. Only a call site knows what its request was doing and which options its own CLI accepts. The printer keeps the hint it already had, which is correct for the pyrit_scan verbs that still reach it, and pyrit_shell no longer reaches it with a timeout at all, so it can never be told to pass an option its parser rejects. A read timeout while starting a run does not mean the run failed to start. The server keeps initializing after the client stops waiting, and it writes the scenario result before it answers, so the outcome is genuinely unknown. Both start paths now say so and point at scenario-history, because retrying blindly can start the run a second time. The results paths point back at scenario-results for the same run instead, since that request is a plain read. Give scenario-history and scenario-results the same treatment. Those are the commands the timeout hints send people to, and they printed the bare exception, so a read timeout there reported the empty message this change exists to fix.
varunj-msft
force-pushed
the
varunj-msft/v1.1.0-Release-Start-Error-Message
branch
from
September 1, 2026 23:55
7547ef9 to
2772ec4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When starting a scenario run failed, the CLI printed
Error starting scenario:with nothing after the colon.httpx.ReadTimeoutcarries no message, sostr(exc)is the empty string, and the one piece of information the user needed — that this was a timeout, not a bad scenario name or a missing target — was the one piece that got dropped.This is not hypothetical. It is the exact output the End to End Tests produce today when the server takes longer to start a run than the client's read timeout allows, and it is a large part of why those failures were so hard to attribute.
Both start-run failure paths, in
pyrit_scanandpyrit_shell, now go through_print_cli_exception, which the CLI already uses elsewhere for exactly this reason. It reports the exception class, falls back torepr()whenstr()is empty, and has a dedicated branch forReadTimeout. The same failure now reads:That version names the failure, gives the flag that changes it, and points at the server logs — the two things you would actually do next. Both call sites are fixed, so the behaviour is the same however the run was started.
Part of the v1.1.0 release wave with #2510, #2511 and #2512.
Tests and Documentation
Two new tests, one per entry point, both covering the
ReadTimeoutcase specifically. They assert that the exception type and the--request-timeouthint both reach the user even though the exception stringifies to the empty string:test_main_start_scenario_read_timeout_reports_type_and_hintintests/unit/cli/test_pyrit_scan.pytest_run_start_failure_read_timeout_reports_type_and_hintintests/unit/cli/test_pyrit_shell.pyThe existing shell test for a generic start failure was updated to assert on the new format,
Error (RuntimeError): nope, which keeps the message content covered rather than only the prefix.Ran
pytest tests/unit/cli/test_pyrit_scan.py tests/unit/cli/test_pyrit_shell.py: 234 passed.No documentation changes. This changes console output only, and no doc or notebook contains this error string. JupyText was not run and is not applicable: no notebooks or code samples are affected.