improve error messages that came up when trying to start a modal agent with a different repo/configuration#904
Open
improve error messages that came up when trying to start a modal agent with a different repo/configuration#904
Conversation
The error reporting for git push --mirror, git clone --mirror, and
remote git push only included e.stderr, which can be empty when git
writes errors to stdout. This made failures like the 0.004s local
git push failure impossible to diagnose ("Failed to push git repo:"
with nothing after the colon).
Now all three error paths include both stdout and stderr plus the
exit code via a shared _format_process_error_details helper.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ProcessError already formats a comprehensive message with command, returncode, stdout, and stderr. No need for a custom helper.
When subprocess.Popen raises OSError (e.g. binary not found, bad cwd), the error message was lost -- ProcessSetupError was created with empty stderr, producing 'Command failed to start None' with no explanation. Now includes str(e) so the actual OS error is visible.
The is_output_already_logged flag was being passed through from the caller, but for setup errors the process never started so nothing could have been logged. This caused stderr=str(e) to be silently suppressed when trace_output was True (which is always the case when running via ConcurrencyGroup).
The previous error "references unknown backend" gave no hint about the cause. Now provides three specific messages: plugin is disabled, another plugin might provide it, or the plugin package may not be installed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ing in the class The previous fix hardcoded is_output_already_logged=False inside ProcessSetupError, silently ignoring the parameter. This is too broad -- the real issue is that the one call site in subprocess_utils.py was passing trace_output (which means "streaming was requested") even though Popen failed before any output was streamed. Fix it at the source. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
evgunter
commented
Mar 20, 2026
Comment on lines
-222
to
+224
| stderr="", | ||
| is_output_already_logged=trace_output, | ||
| stderr=str(e), | ||
| # Popen failed, so no output was ever streamed regardless of trace_output. | ||
| is_output_already_logged=False, |
evgunter
changed the title
evgunter
changed the title
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
1 participant
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.
Summary
Multiple layers of error reporting were silently discarding diagnostic information, making subprocess failures impossible to debug.
host.py-- git push/clone errors only showede.stderrProcessError.__str__()already includes command, returncode, stdout, and stderr. The error handlers were manually extracting juste.stderr, losing everything else. Changed tostr(e). For theCommandResultpath (remote push), stderr and stdout are now combined.subprocess_utils.py--ProcessSetupErrorswallowed the OS errorWhen
subprocess.PopenraisesOSError(e.g. binary not found, bad cwd), two things went wrong:stderr="")is_output_already_loggedwas passed through asTruefrom the caller (since trace output is always enabled), suppressing the output section even when nothing was ever loggedFixed by including
str(e)in stderr and hardcodingis_output_already_logged=False(the process never started, so nothing was streamed).loader.py-- better error when backend plugin is missingWhen a provider config references an unknown backend, the error message now distinguishes between: the specific plugin being disabled, some other plugin being disabled, or no plugins disabled (suggesting the package isn't installed).
Context
Found when debugging a Modal agent creation failure where
git push --mirrorfailed in 0.004s with "Failed to push git repo:" and nothing after the colon. After fixing the error reporting, the real error was revealed:[Errno 2] No such file or directory: git-- the subprocess PATH was stripped before mng started (auv toolenvironment issue, not an mng bug).Test plan