Skip to content

Avoid returning from finally in _do_run. #13

@lpiwowar

Description

@lpiwowar

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid returning from finally in _do_run.

At Line 422, returning inside finally can silently swallow unexpected exceptions and makes runtime failures much harder to detect.

Suggested fix
 def _do_run(self, cmd: list[str]) -> tuple[int, str, str]:
     self._clean_stds()
+    return_code = 0
     try:
         return_code = super().run(cmd)
     except (SystemExit, Exception) as e:
         return_code = getattr(e, "code", 1)
         msg = getattr(e, "msg", str(e))
         logger.debug(
             f"Failure running command: {cmd} with code: {return_code} and message: {msg}"
         )
     finally:
         stdout = self.stdout.getvalue()
         stderr = self.stderr.getvalue()
         self._clean_stds()
-        return return_code, stdout, stderr
+    return return_code, stdout, stderr
🧰 Tools
🪛 Ruff (0.15.14)

[warning] 422-422: return inside finally blocks cause exceptions to be silenced

(B012)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/rhos_ls_mcps/osc.py` around lines 418 - 423, The _do_run function
currently returns from inside the finally block (return return_code, stdout,
stderr), which can swallow exceptions; change the flow so finally only performs
cleanup (call self._clean_stds() and capture stdout/stderr into locals) and do
not return there. Instead assign stdout/stderr to local variables (e.g., stdout
= self.stdout.getvalue(); stderr = self.stderr.getvalue()), perform
self._clean_stds() in finally, and move the actual return (return return_code,
stdout, stderr) to the end of _do_run (after the try/except/finally) so any
exceptions propagate normally; reference symbols: _do_run,
self.stdout.getvalue(), self.stderr.getvalue(), self._clean_stds(), return_code.

✅ Addressed in commit 6ded951

Originally posted by @coderabbitai[bot] in #11 (comment)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions