Skip to content

Commit d5d1b48

Browse files
yeldarbyclaude
andcommitted
Fix error handling in model and train handlers
- model list: wrap SDK calls in try/except for clean error output - model/train: extract message from JSON-encoded API errors to prevent double-encoding in --json mode - Add tests for error message extraction and model list 404 case Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f30356e commit d5d1b48

File tree

4 files changed

+135
-6
lines changed

4 files changed

+135
-6
lines changed

roboflow/cli/handlers/model.py

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,31 @@
22

33
from __future__ import annotations
44

5+
import json as _json
56
from typing import TYPE_CHECKING
67

78
if TYPE_CHECKING:
89
import argparse
910

1011

12+
def _extract_error_message(raw: str) -> str:
13+
"""Extract a human-readable message from a potentially JSON-encoded error string."""
14+
text = raw.strip()
15+
colon_idx = text.find(": {")
16+
if colon_idx > 0 and colon_idx < 5:
17+
text = text[colon_idx + 2 :]
18+
try:
19+
parsed = _json.loads(text)
20+
if isinstance(parsed, dict):
21+
err = parsed.get("error", parsed)
22+
if isinstance(err, dict):
23+
return str(err.get("message") or err.get("hint") or err)
24+
return str(err)
25+
except (ValueError, TypeError):
26+
pass
27+
return raw
28+
29+
1130
def register(subparsers: argparse._SubParsersAction) -> None: # type: ignore[type-arg]
1231
"""Register ``model`` subcommand and its verbs."""
1332
model_parser = subparsers.add_parser("model", help="Manage trained models")
@@ -96,11 +115,19 @@ def _list_models(args: argparse.Namespace) -> None:
96115
return
97116

98117
api_key = args.api_key or None
99-
rf = roboflow.Roboflow(api_key=api_key)
100-
workspace = rf.workspace(workspace_url)
101-
project = workspace.project(project_slug)
102118

103-
versions = project.versions()
119+
try:
120+
from roboflow.cli._output import suppress_sdk_output
121+
122+
with suppress_sdk_output(args):
123+
rf = roboflow.Roboflow(api_key=api_key)
124+
workspace = rf.workspace(workspace_url)
125+
project = workspace.project(project_slug)
126+
versions = project.versions()
127+
except Exception as exc:
128+
output_error(args, _extract_error_message(str(exc)), exit_code=3)
129+
return
130+
104131
models = []
105132
for v in versions:
106133
if v.model:
@@ -148,7 +175,7 @@ def _get_model(args: argparse.Namespace) -> None:
148175
else:
149176
data = rfapi.get_project(api_key, workspace_url, project_slug)
150177
except rfapi.RoboflowError as exc:
151-
output_error(args, str(exc), exit_code=3)
178+
output_error(args, _extract_error_message(str(exc)), exit_code=3)
152179
return
153180

154181
output(args, data, text=json.dumps(data, indent=2, default=str))

roboflow/cli/handlers/train.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,33 @@
22

33
from __future__ import annotations
44

5+
import json as _json
56
from typing import TYPE_CHECKING
67

78
if TYPE_CHECKING:
89
import argparse
910

1011

12+
def _extract_error_message(raw: str) -> str:
13+
"""Extract a human-readable message from a potentially JSON-encoded error string."""
14+
# Strip status code prefix like "404: {...}"
15+
text = raw.strip()
16+
colon_idx = text.find(": {")
17+
if colon_idx > 0 and colon_idx < 5:
18+
text = text[colon_idx + 2 :]
19+
20+
try:
21+
parsed = _json.loads(text)
22+
if isinstance(parsed, dict):
23+
err = parsed.get("error", parsed)
24+
if isinstance(err, dict):
25+
return str(err.get("message") or err.get("hint") or err)
26+
return str(err)
27+
except (ValueError, TypeError):
28+
pass
29+
return raw
30+
31+
1132
def register(subparsers: argparse._SubParsersAction) -> None: # type: ignore[type-arg]
1233
"""Register ``train`` subcommand and its verbs."""
1334
train_parser = subparsers.add_parser("train", help="Train a model")
@@ -104,7 +125,7 @@ def _start(args: argparse.Namespace) -> None:
104125
epochs=args.epochs,
105126
)
106127
except rfapi.RoboflowError as exc:
107-
output_error(args, str(exc))
128+
output_error(args, _extract_error_message(str(exc)))
108129
return
109130

110131
data = {

tests/cli/test_model_handler.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,5 +203,64 @@ def test_upload_no_project_errors(self, mock_rf_cls: MagicMock) -> None:
203203
_upload_model(args)
204204

205205

206+
class TestExtractErrorMessage(unittest.TestCase):
207+
"""Test _extract_error_message helper."""
208+
209+
def test_plain_string(self) -> None:
210+
from roboflow.cli.handlers.model import _extract_error_message
211+
212+
self.assertEqual(_extract_error_message("something broke"), "something broke")
213+
214+
def test_json_with_nested_error(self) -> None:
215+
from roboflow.cli.handlers.model import _extract_error_message
216+
217+
raw = '{"error": {"message": "Unsupported request"}}'
218+
self.assertEqual(_extract_error_message(raw), "Unsupported request")
219+
220+
def test_json_with_string_error(self) -> None:
221+
from roboflow.cli.handlers.model import _extract_error_message
222+
223+
raw = '{"error": "Not found"}'
224+
self.assertEqual(_extract_error_message(raw), "Not found")
225+
226+
227+
class TestModelListError(unittest.TestCase):
228+
"""Test _list_models handles API errors cleanly."""
229+
230+
def _make_args(self, **kwargs: object) -> types.SimpleNamespace:
231+
defaults = {
232+
"json": True,
233+
"api_key": "test-key",
234+
"workspace": "test-ws",
235+
"project": "nonexistent-project",
236+
}
237+
defaults.update(kwargs)
238+
return types.SimpleNamespace(**defaults)
239+
240+
@patch("roboflow.Roboflow")
241+
def test_list_models_project_not_found(self, mock_rf_cls: MagicMock) -> None:
242+
from roboflow.cli.handlers.model import _list_models
243+
244+
mock_workspace = MagicMock()
245+
mock_workspace.project.side_effect = RuntimeError("Project not found")
246+
mock_rf = MagicMock()
247+
mock_rf.workspace.return_value = mock_workspace
248+
mock_rf_cls.return_value = mock_rf
249+
250+
args = self._make_args()
251+
buf = io.StringIO()
252+
old_stderr = sys.stderr
253+
sys.stderr = buf
254+
try:
255+
with self.assertRaises(SystemExit) as ctx:
256+
_list_models(args)
257+
self.assertEqual(ctx.exception.code, 3)
258+
finally:
259+
sys.stderr = old_stderr
260+
261+
result = json.loads(buf.getvalue())
262+
self.assertIn("error", result)
263+
264+
206265
if __name__ == "__main__":
207266
unittest.main()

tests/cli/test_train_handler.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,28 @@ def test_start_no_api_key(self, _mock_key: MagicMock) -> None:
144144
_start(args)
145145
self.assertEqual(ctx.exception.code, 2)
146146

147+
@patch("roboflow.adapters.rfapi.start_version_training")
148+
def test_start_json_error_not_double_encoded(self, mock_train: MagicMock) -> None:
149+
from roboflow.adapters.rfapi import RoboflowError
150+
from roboflow.cli.handlers.train import _start
151+
152+
# Simulate API returning a JSON error string
153+
mock_train.side_effect = RoboflowError('{"error": {"message": "Unsupported request"}}')
154+
155+
args = self._make_args(json=True)
156+
buf = io.StringIO()
157+
old_stderr = sys.stderr
158+
sys.stderr = buf
159+
try:
160+
with self.assertRaises(SystemExit):
161+
_start(args)
162+
finally:
163+
sys.stderr = old_stderr
164+
165+
result = json.loads(buf.getvalue())
166+
# Should be a clean string, not double-encoded JSON
167+
self.assertEqual(result["error"], "Unsupported request")
168+
147169

148170
if __name__ == "__main__":
149171
unittest.main()

0 commit comments

Comments
 (0)