fix(login): narrow exception types and cover cleanup path
lint / lint (push) Successful in 43s
test / integration-docker (pull_request) Successful in 8s
tracker-policy-pr / check-pr (pull_request) Successful in 5s
test / unit (pull_request) Successful in 31s
test / stage-firecracker-inputs (pull_request) Successful in 5s
test / build-infra (pull_request) Successful in 3m37s
test / integration-firecracker (pull_request) Successful in 1m39s
test / coverage (pull_request) Successful in 2m1s
test / publish-infra (pull_request) Has been skipped

Replaces broad `except Exception` with specific types that reflect the
actual failure modes:
- _save_credentials: `except OSError` (IO-only path; re-raises for cleanup)
- _post error handler: `except (OSError, ValueError)` (network + bad JSON)
- poll-loop: `except (OSError, ValueError)` (network + bad JSON)

Also:
- Simplify _fake_get to use next(..., default) — removes uncovered
  StopIteration branch
- Add encoding="utf-8" to open() in test_approved_flow_returns_0
- Add test_cleanup_on_write_failure to cover the except OSError block
  in _save_credentials

All files score 10.00/10 on pylint (fail-under=10) and 0 errors on
pyright strict.
This commit is contained in:
2026-07-21 04:27:30 +00:00
parent 67cb51352a
commit 2915706659
2 changed files with 17 additions and 9 deletions
+3 -3
View File
@@ -92,7 +92,7 @@ def _save_credentials(
with os.fdopen(fd, "w") as f:
f.write(content)
os.replace(tmp, path)
except Exception:
except OSError:
try:
tmp.unlink()
except OSError:
@@ -118,7 +118,7 @@ def cmd_login(argv: list[str]) -> int:
try:
resp = _post(f"{console_url}/api/v1/hosts/authorize", {"label": label})
except Exception as exc:
except (OSError, ValueError) as exc:
sys.stderr.write(f"bb login: failed to start authorization: {exc}\n")
return 1
@@ -143,7 +143,7 @@ def cmd_login(argv: list[str]) -> int:
code, result = _get(
f"{console_url}/api/v1/hosts/authorize/{device_code}"
)
except Exception:
except (OSError, ValueError):
continue
if code == 410:
+14 -6
View File
@@ -105,6 +105,18 @@ class TestSaveCredentials(unittest.TestCase):
self.assertEqual(oct(tmp_perms_at_replace[0]), oct(0o600))
self.assertEqual(oct(path.stat().st_mode & 0o777), oct(0o600))
def test_cleanup_on_write_failure(self) -> None:
"""Temp file is removed and no credentials remain if replace fails."""
from bot_bottle.cli.login import _save_credentials
with tempfile.TemporaryDirectory() as tmp:
with patch.dict(os.environ, {"BOT_BOTTLE_ROOT": tmp}):
with patch("os.replace", side_effect=OSError("disk full")):
with self.assertRaises(OSError):
_save_credentials("http://c", "hid", "at", "rt")
leftovers = [f for f in os.listdir(tmp) if f.startswith(".console-")]
self.assertEqual(leftovers, [])
class TestCmdLoginMissingUrl(unittest.TestCase):
def test_help_returns_0(self) -> None:
@@ -160,11 +172,7 @@ class TestCmdLoginFlow(unittest.TestCase):
return start_resp
def _fake_get(_url: str) -> tuple[int, dict[str, Any]]:
try:
resp = next(poll_iter)
except StopIteration:
return 200, {"status": "pending"}
return 200, resp
return 200, next(poll_iter, {"status": "pending"})
with patch.dict(os.environ, {"BOT_BOTTLE_ROOT": tmp}):
with patch("bot_bottle.cli.login._post", side_effect=_fake_post):
@@ -184,7 +192,7 @@ class TestCmdLoginFlow(unittest.TestCase):
[{"status": "pending"}, approved], tmp
)
self.assertEqual(result, 0)
with open(os.path.join(tmp, "console.json")) as f:
with open(os.path.join(tmp, "console.json"), encoding="utf-8") as f:
creds = json.loads(f.read())
self.assertEqual(creds["host_id"], "hid")