Skip to content

Commit fc43f1a

Browse files
holy-pharaohclaude
andcommitted
fix: Improve Jira export error handling, env var warnings, and CLI test coverage
- Surface Jira API error body (errorMessages, errors fields) as clean ValueError instead of raw HTTPError traceback on 4xx/5xx responses - Warn when ${VAR} reference in workspace.yaml resolves to unset env var instead of silently returning None - Add 7 new tests: _raise_for_status error paths, CLI config wiring, clean error display in terminal Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 322dde1 commit fc43f1a

File tree

3 files changed

+166
-3
lines changed

3 files changed

+166
-3
lines changed

corbell/core/export/jira.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ def export_tasks(self, tasks_yaml_path: Path | str) -> List[Dict[str, Any]]:
100100
data=json.dumps(payload),
101101
timeout=30,
102102
)
103-
resp.raise_for_status()
103+
self._raise_for_status(resp)
104104
result = resp.json()
105105

106106
issue_key = result.get("key", "")
@@ -134,6 +134,20 @@ def _validate_credentials(self) -> None:
134134
"Set env vars or add 'integrations.jira.*' to workspace.yaml."
135135
)
136136

137+
def _raise_for_status(self, resp: Any) -> None:
138+
"""Raise ValueError with a clean Jira error message on 4xx/5xx."""
139+
if resp.status_code < 400:
140+
return
141+
try:
142+
body = resp.json()
143+
messages = body.get("errorMessages", [])
144+
errors = body.get("errors", {})
145+
parts = list(messages) + [f"{k}: {v}" for k, v in errors.items()]
146+
detail = "; ".join(parts) if parts else resp.text[:300]
147+
except Exception:
148+
detail = resp.text[:300]
149+
raise ValueError(f"Jira API {resp.status_code}: {detail}")
150+
137151
def _auth_headers(self) -> Dict[str, str]:
138152
token = base64.b64encode(f"{self.email}:{self.api_token}".encode()).decode()
139153
return {"Authorization": f"Basic {token}"}

corbell/core/workspace.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,12 +224,21 @@ def _expand_env(value: Any) -> Any:
224224
"""Recursively expand ${VAR} references in dict/list/str values.
225225
226226
- ``${VAR}`` → value of env var VAR, or None if not set
227+
(a warning is emitted when the var is missing)
227228
- Any other string → used as-is (literal value)
228229
"""
229230
if isinstance(value, str):
230231
if value.startswith("${") and value.endswith("}"):
231232
var = value[2:-1]
232-
return os.environ.get(var, None) # None when env var is not set
233+
resolved = os.environ.get(var)
234+
if resolved is None:
235+
import warnings
236+
warnings.warn(
237+
f"Environment variable '{var}' is referenced in workspace.yaml but is not set.",
238+
UserWarning,
239+
stacklevel=2,
240+
)
241+
return resolved
233242
return value
234243
if isinstance(value, dict):
235244
return {k: _expand_env(v) for k, v in value.items()}

tests/test_jira_export.py

Lines changed: 141 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
"""Tests for the Jira exporter."""
1+
"""Tests for the Jira exporter and CLI command."""
22

33
from __future__ import annotations
44

@@ -80,6 +80,7 @@ def tasks_yaml_no_files(tmp_path) -> Path:
8080

8181
def _mock_response(key: str = "ENG-1") -> MagicMock:
8282
resp = MagicMock()
83+
resp.status_code = 201
8384
resp.json.return_value = {"key": key, "id": "10001"}
8485
resp.raise_for_status.return_value = None
8586
return resp
@@ -347,3 +348,142 @@ def test_jira_workspace_config_overrides_env(tmp_path):
347348
assert jira.api_token == "secret-token"
348349
assert jira.project_key == "ACME"
349350
assert jira.issue_type == "Story"
351+
352+
353+
# ─── Jira API error handling ──────────────────────────────────────────────────
354+
355+
def test_raise_for_status_surfaces_jira_error_messages():
356+
exporter = JiraExporter(**VALID_CREDS)
357+
resp = MagicMock()
358+
resp.status_code = 400
359+
resp.json.return_value = {
360+
"errorMessages": ["Project 'XYZ' does not exist."],
361+
"errors": {},
362+
}
363+
with pytest.raises(ValueError, match="Project 'XYZ' does not exist"):
364+
exporter._raise_for_status(resp)
365+
366+
367+
def test_raise_for_status_surfaces_field_errors():
368+
exporter = JiraExporter(**VALID_CREDS)
369+
resp = MagicMock()
370+
resp.status_code = 400
371+
resp.json.return_value = {
372+
"errorMessages": [],
373+
"errors": {"issuetype": "Issue type is required."},
374+
}
375+
with pytest.raises(ValueError, match="issuetype"):
376+
exporter._raise_for_status(resp)
377+
378+
379+
def test_raise_for_status_401_fallback_to_text():
380+
exporter = JiraExporter(**VALID_CREDS)
381+
resp = MagicMock()
382+
resp.status_code = 401
383+
resp.json.side_effect = Exception("not json")
384+
resp.text = "Unauthorized"
385+
with pytest.raises(ValueError, match="401"):
386+
exporter._raise_for_status(resp)
387+
388+
389+
def test_raise_for_status_2xx_does_not_raise():
390+
exporter = JiraExporter(**VALID_CREDS)
391+
resp = MagicMock()
392+
resp.status_code = 201
393+
exporter._raise_for_status(resp) # should not raise
394+
395+
396+
def test_export_tasks_surfaces_jira_error(tasks_yaml):
397+
"""HTTPError from Jira should surface as a clean ValueError, not a traceback."""
398+
exporter = JiraExporter(**VALID_CREDS)
399+
400+
bad_resp = MagicMock()
401+
bad_resp.status_code = 400
402+
bad_resp.json.return_value = {"errorMessages": ["Issue type 'Task' not found."], "errors": {}}
403+
404+
with patch("requests.Session.post", return_value=bad_resp):
405+
with pytest.raises(ValueError, match="Issue type 'Task' not found"):
406+
exporter.export_tasks(tasks_yaml)
407+
408+
409+
# ─── CLI command ──────────────────────────────────────────────────────────────
410+
411+
def test_cli_export_jira_reads_config_from_workspace(tmp_path, tasks_yaml):
412+
"""CLI `export jira` must pass workspace.yaml values to JiraExporter."""
413+
from typer.testing import CliRunner
414+
from corbell.cli.commands.export import app
415+
416+
# Create a minimal workspace with Jira config
417+
ws_dir = tmp_path / "corbell-data"
418+
ws_dir.mkdir()
419+
(ws_dir / "workspace.yaml").write_text(
420+
"""\
421+
version: "1"
422+
integrations:
423+
jira:
424+
url: https://test.atlassian.net
425+
email: test@test.com
426+
api_token: test-token
427+
project_key: TEST
428+
issue_type: Task
429+
""",
430+
encoding="utf-8",
431+
)
432+
433+
captured = {}
434+
435+
def fake_export(self, path):
436+
captured["url"] = self.url
437+
captured["email"] = self.email
438+
captured["api_token"] = self.api_token
439+
captured["project_key"] = self.project_key
440+
return [{"issue_key": "TEST-1", "title": "t", "url": "u"}]
441+
442+
runner = CliRunner()
443+
with patch("corbell.core.export.jira.JiraExporter.export_tasks", fake_export):
444+
result = runner.invoke(
445+
app,
446+
["jira", str(tasks_yaml), "--workspace", str(tmp_path)],
447+
)
448+
449+
assert result.exit_code == 0, result.output
450+
assert captured["url"] == "https://test.atlassian.net"
451+
assert captured["email"] == "test@test.com"
452+
assert captured["api_token"] == "test-token"
453+
assert captured["project_key"] == "TEST"
454+
455+
456+
def test_cli_export_jira_shows_clean_error_on_bad_credentials(tmp_path, tasks_yaml):
457+
"""CLI must show a clean error message, not a Python traceback."""
458+
from typer.testing import CliRunner
459+
from corbell.cli.commands.export import app
460+
461+
ws_dir = tmp_path / "corbell-data"
462+
ws_dir.mkdir()
463+
(ws_dir / "workspace.yaml").write_text(
464+
"""\
465+
version: "1"
466+
integrations:
467+
jira:
468+
url: https://test.atlassian.net
469+
email: test@test.com
470+
api_token: bad-token
471+
project_key: TEST
472+
issue_type: Task
473+
""",
474+
encoding="utf-8",
475+
)
476+
477+
def fake_export(self, path):
478+
raise ValueError("Jira API 401: Unauthorized")
479+
480+
runner = CliRunner()
481+
with patch("corbell.core.export.jira.JiraExporter.export_tasks", fake_export):
482+
result = runner.invoke(
483+
app,
484+
["jira", str(tasks_yaml), "--workspace", str(tmp_path)],
485+
)
486+
487+
assert result.exit_code == 1
488+
assert "Jira API 401" in result.output
489+
assert "Traceback" not in result.output

0 commit comments

Comments
 (0)