Skip to content

fix: handle TimeoutExpired and OSError in dispatch_command() - #3921

Open
Quratulain-bilal wants to merge 1 commit into
github:mainfrom
Quratulain-bilal:fix/dispatch-timeout-handler
Open

fix: handle TimeoutExpired and OSError in dispatch_command()#3921
Quratulain-bilal wants to merge 1 commit into
github:mainfrom
Quratulain-bilal:fix/dispatch-timeout-handler

Conversation

@Quratulain-bilal

Copy link
Copy Markdown
Contributor

Problem

subprocess.run() with timeout parameter can raise TimeoutExpired if the timeout elapses. Also catch OSError for cases where the binary is not found or not executable.

Fix

Added try/except blocks to handle both exception types in both base and copilot dispatch_command().

subprocess.run() with timeout parameter can raise TimeoutExpired if the
timeout elapses. Also catch OSError for cases where the binary is not
found or not executable (TOCTOU race between shutil.which and run).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds structured handling for command timeouts and execution failures during captured CLI dispatch.

Changes:

  • Maps timeouts to exit code 124.
  • Converts OSError failures into dispatch results.
Show a summary per file
File Description
src/specify_cli/integrations/base.py Handles captured dispatch exceptions.
src/specify_cli/integrations/copilot/__init__.py Adds equivalent Copilot handling.

Review details

馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Balanced

"stdout": "",
"stderr": f"Command timed out after {timeout}s",
}
except OSError as exc:
"stdout": "",
"stderr": f"Command timed out after {timeout}s",
}
except OSError as exc:
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.

3 participants