Skip to content

Commit ffc6d55

Browse files
authored
Use predefined exception instead of arbitrary exit code in command modules (#995)
## Summary Fixes #509. Replaces bare `sys.exit(N)` / `ctx.exit(N)` / `raise SystemExit(N)` calls in command modules with predefined, typed exceptions so exit codes are managed consistently instead of scattered as magic numbers. No exit-code definitions change — codes 1/2/3/4 are preserved exactly. ## Changes - Added three typed exceptions in `utils/cli.py`: - `ModelLoadError` → exit 3 (model fails to load on device/EP) - `InferenceError` → exit 4 (prediction fails at runtime) - `PartialSupportError` → exit 1 (valid negative result; raised silently, no `Error:` prefix) - `ModelLoadError`/`InferenceError` override `show()` to print the message verbatim, preserving existing stderr output. - `analyze.py`: removed `sys` import; all exits now raise `UsageError` (2) or `PartialSupportError` (1); added a re-raise guard so intentional exits aren't relabeled "Analysis failed". - `run.py`: parse/format/file errors → `UsageError` (2); load → `ModelLoadError` (3); inference → `InferenceError` (4); success no-input path keeps `ctx.exit(0)`. - `perf.py` / `serve.py`: install/load failures → `ClickException` (1), with a re-raise guard in `perf`. ## Exit code contract (unchanged) | Code | Meaning | |------|---------| | 0 | success | | 1 | negative result (partial support) | | 2 | usage error | | 3 | model load failure | | 4 | inference failure | ## Testing - Existing `run` tests asserting exit 3/4 are unchanged and still pass — behavior is fully guarded. - 261 unit tests pass (`run`, `run_spec`, `perf_cli`, `optracing`, `serve`); no type/lint errors.
1 parent c89fa91 commit ffc6d55

5 files changed

Lines changed: 100 additions & 66 deletions

File tree

src/winml/modelkit/commands/analyze.py

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import logging
1818
import os
1919
import re
20-
import sys
2120
from pathlib import Path
2221
from typing import TYPE_CHECKING, Any, Literal, cast
2322

@@ -840,8 +839,7 @@ def analyze(
840839

841840
# Validate model
842841
if not model.exists():
843-
logger.error("ONNX model file not found: %s", model)
844-
sys.exit(2)
842+
raise click.UsageError(f"ONNX model file not found: {model}")
845843

846844
from ..analyze.utils.ep_utils import (
847845
has_any_rule_data,
@@ -891,15 +889,14 @@ def analyze(
891889
"Resolved absolute path(s) from %s: (none)",
892890
WINMLCLI_RULES_DIR_FOR_DEBUG_ENV,
893891
)
894-
sys.exit(2)
892+
raise click.UsageError("--debug rules directory not configured.")
895893

896894
search_dirs = get_runtime_rules_search_dirs()
897895
if not has_any_rule_data():
898896
searched = ", ".join(str(p) for p in search_dirs) if search_dirs else "(none)"
899-
logger.error("No runtime rule parquet files were found.")
900897
logger.error("Please reinstall winml-cli, or manually download rule parquet files.")
901898
logger.error("Searched directories: %s", searched)
902-
sys.exit(2)
899+
raise click.UsageError("No runtime rule parquet files were found.")
903900

904901
# Resolve the EP/device selection. `all` keeps the full rule-data-backed
905902
# set (fan-out, unchanged). `auto` resolves to a single best target from
@@ -921,8 +918,7 @@ def analyze(
921918
try:
922919
resolved_device, _ = resolve_device(device="auto", ep=ep_hint)
923920
except (ValueError, RuntimeError) as e:
924-
logger.error("Could not auto-select a device: %s", e)
925-
sys.exit(2)
921+
raise click.UsageError(f"Could not auto-select a device: {e}") from e
926922
devices = [resolved_device]
927923
elif device is not None:
928924
devices = [device]
@@ -956,12 +952,12 @@ def analyze(
956952
# of raising an unguarded IndexError on ``devices[0]``.
957953
ref_device = devices[0] if devices else None
958954
if not ref_device:
959-
logger.error("No device context available for EP auto-resolution.")
960-
sys.exit(2)
955+
raise click.UsageError("No device context available for EP auto-resolution.")
961956
compatible_eps = resolve_eps(ref_device)
962957
if not compatible_eps:
963-
logger.error("No execution provider is available for device '%s'.", ref_device)
964-
sys.exit(2)
958+
raise click.UsageError(
959+
f"No execution provider is available for device '{ref_device}'."
960+
)
965961
eps = [compatible_eps[0]]
966962
else:
967963
# ep is a specific EP or alias
@@ -989,8 +985,7 @@ def analyze(
989985
local_pairs = set(_get_local_ep_device_pairs())
990986

991987
if not execution_pairs:
992-
logger.error("No EP/device combination matched the current selection.")
993-
sys.exit(2)
988+
raise click.UsageError("No EP/device combination matched the current selection.")
994989

995990
logger.info("Analyzing model: %s", model)
996991
logger.info(
@@ -1441,16 +1436,19 @@ def on_node_result(pattern_runtime: PatternRuntime) -> None:
14411436

14421437
# Exit code: 0 = fully supported, 1 = partial support
14431438
overall_supported = all(run_result.is_fully_supported() for run_result in analysis_results)
1444-
sys.exit(0 if overall_supported else 1)
1439+
if not overall_supported:
1440+
raise cli_utils.PartialSupportError
14451441

14461442
except FileNotFoundError as e:
1447-
logger.error("File not found: %s", e)
1448-
sys.exit(2)
1443+
raise click.UsageError(f"File not found: {e}") from e
1444+
except (click.exceptions.Exit, click.ClickException):
1445+
# Exit/click exceptions are intentional control flow; re-raise so the
1446+
# catch-all below doesn't relabel them as "Analysis failed".
1447+
raise
14491448
except Exception as e:
1450-
logger.error("Analysis failed: %s", e)
14511449
if verbose:
14521450
logger.exception("Full traceback:")
1453-
sys.exit(2)
1451+
raise click.UsageError(f"Analysis failed: {e}") from e
14541452

14551453

14561454
__all__ = ["analyze"]

src/winml/modelkit/commands/perf.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1825,13 +1825,12 @@ def perf(
18251825
if not is_profiling_available(
18261826
benchmark.resolved_ep, benchmark.resolved_device, op_tracing
18271827
):
1828-
console.print(
1829-
"[red]Error:[/red] Op-tracing is only supported for the QNN EP "
1828+
raise click.ClickException(
1829+
"Op-tracing is only supported for the QNN EP "
18301830
"on NPU at the 'basic' level "
18311831
f"(resolved EP={benchmark.resolved_ep}, "
18321832
f"device={benchmark.resolved_device}, level={op_tracing})."
18331833
)
1834-
raise SystemExit(1)
18351834

18361835
from ..optracing import (
18371836
display_op_trace_report,
@@ -1848,20 +1847,18 @@ def perf(
18481847
if onnx_for_trace is None:
18491848
raise AttributeError("benchmark._model not initialized")
18501849
except AttributeError:
1851-
console.print(
1852-
"[red]Error:[/red] Could not determine ONNX model path for op-tracing"
1853-
)
1854-
raise SystemExit(1) from None
1850+
raise click.ClickException(
1851+
"Could not determine ONNX model path for op-tracing"
1852+
) from None
18551853

18561854
output_dir = output.parent if output else Path()
18571855

18581856
# Look up tracer via registry (EP-agnostic).
18591857
tracer_cls = get_tracer("QNNExecutionProvider", op_tracing)
18601858
if tracer_cls is None:
1861-
console.print(
1862-
f"[red]Error:[/red] No tracer registered for QNN EP at level '{op_tracing}'"
1859+
raise click.ClickException(
1860+
f"No tracer registered for QNN EP at level '{op_tracing}'"
18631861
)
1864-
raise SystemExit(1)
18651862

18661863
profiler = tracer_cls(
18671864
onnx_for_trace,
@@ -1887,6 +1884,10 @@ def perf(
18871884
# the convention used by Click for argument problems.
18881885
raise click.UsageError(f"Model not found: {e}") from e
18891886

1887+
except click.ClickException:
1888+
# Click exceptions are already intentional control flow; re-raise so
1889+
# the catch-all below doesn't relabel them as "Benchmark failed".
1890+
raise
18901891
except Exception as e:
18911892
if verbose:
18921893
logger.exception("Benchmark failed")

src/winml/modelkit/commands/run.py

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,18 @@ def run(
519519
Uses embedded inference by default. Pass ``--connect`` to route
520520
through a running ``winml serve`` instance instead.
521521
522+
Exit Codes:
523+
524+
0: Success
525+
526+
1: General error
527+
528+
2: Usage error — invalid input or arguments
529+
530+
3: Model load failure
531+
532+
4: Inference failure
533+
522534
Examples:
523535
\b
524536
# Image classification (shortcut)
@@ -547,17 +559,15 @@ def run(
547559
pipeline_kwargs: dict[str, Any] = {}
548560
for p in params:
549561
if "=" not in p:
550-
click.echo(f"Error: invalid --param format: '{p}'. Use KEY=VALUE.", err=True)
551-
ctx.exit(2)
562+
raise click.UsageError(f"invalid --param format: '{p}'. Use KEY=VALUE.")
552563
k, v = p.split("=", 1)
553564
pipeline_kwargs[k] = _parse_param_value(v)
554565

555566
# Parse --input entries (raw strings, coerced after model load)
556567
raw_inputs: dict[str, str] = {}
557568
for inp in input_args:
558569
if "=" not in inp:
559-
click.echo(f"Error: invalid --input format: '{inp}'. Use NAME=VALUE.", err=True)
560-
ctx.exit(2)
570+
raise click.UsageError(f"invalid --input format: '{inp}'. Use NAME=VALUE.")
561571
k, v = inp.split("=", 1)
562572
raw_inputs[k] = v
563573

@@ -566,17 +576,14 @@ def run(
566576
for fp in files:
567577
file_path = Path(fp)
568578
if not file_path.exists() or not file_path.is_file():
569-
click.echo(f"Error: file not found: {fp}", err=True)
570-
ctx.exit(2)
579+
raise click.UsageError(f"file not found: {fp}")
571580
file_bytes_list.append(file_path.read_bytes())
572581

573582
if len(file_bytes_list) > 1:
574-
click.echo(
575-
f"Error: --file accepts only one file (got {len(file_bytes_list)}). "
576-
"Use --input for multiple file inputs (e.g. -I image_0=@a.jpg -I image_1=@b.jpg).",
577-
err=True,
583+
raise click.UsageError(
584+
f"--file accepts only one file (got {len(file_bytes_list)}). "
585+
"Use --input for multiple file inputs (e.g. -I image_0=@a.jpg -I image_1=@b.jpg)."
578586
)
579-
ctx.exit(2)
580587

581588
# Check if any input was provided
582589
has_inputs = bool(file_bytes_list) or text is not None or bool(raw_inputs)
@@ -619,8 +626,7 @@ def run(
619626
try:
620627
engine.load_schema_only(model, task=task, device=device, ep=ep)
621628
except (OSError, ValueError, RuntimeError) as exc:
622-
click.echo(f"Error loading model: {exc}", err=True)
623-
ctx.exit(3)
629+
raise cli_utils.ModelLoadError(f"Error loading model: {exc}") from exc
624630
_print_schema(engine, output_format=output_format, output_path=output)
625631
return
626632

@@ -638,8 +644,7 @@ def run(
638644
allow_unsupported_nodes=allow_unsupported_nodes,
639645
)
640646
except (OSError, ValueError, RuntimeError) as exc:
641-
click.echo(f"Error loading model: {exc}", err=True)
642-
ctx.exit(3)
647+
raise cli_utils.ModelLoadError(f"Error loading model: {exc}") from exc
643648

644649
# No inputs: print hint and exit
645650
if not has_inputs:
@@ -651,33 +656,28 @@ def run(
651656
try:
652657
coerced_inputs = _coerce_inputs(raw_inputs, schema)
653658
except click.ClickException as exc:
654-
click.echo(f"Error: {exc.format_message()}", err=True)
655-
ctx.exit(2)
659+
raise click.UsageError(exc.format_message()) from exc
656660

657661
# Merge --file/--text shortcuts with --input
658662
try:
659663
inputs = _resolve_shortcuts(file_bytes_list, text, coerced_inputs, schema)
660664
except click.ClickException as exc:
661-
click.echo(f"Error: {exc.format_message()}", err=True)
662-
ctx.exit(2)
665+
raise click.UsageError(exc.format_message()) from exc
663666

664667
# Check input / -P collision (after shortcuts are resolved so that
665668
# --file and --text shortcut keys are included in the check)
666669
collision = set(inputs.keys()) & set(pipeline_kwargs.keys())
667670
if collision:
668671
key = sorted(collision)[0]
669-
click.echo(
670-
f"Error: '{key}' specified as both input and -P. "
671-
f"Use --input for model inputs and -P for pipeline parameters.",
672-
err=True,
672+
raise click.UsageError(
673+
f"'{key}' specified as both input and -P. "
674+
"Use --input for model inputs and -P for pipeline parameters."
673675
)
674-
ctx.exit(2)
675676

676677
try:
677678
prediction = engine.predict(inputs=inputs, **pipeline_kwargs)
678679
except (ValueError, TypeError, RuntimeError, OSError) as exc:
679-
click.echo(f"Error during inference: {exc}", err=True)
680-
ctx.exit(4)
680+
raise cli_utils.InferenceError(f"Error during inference: {exc}") from exc
681681

682682
_print_result(prediction.model_dump(), output_format=output_format, output_path=output)
683683

src/winml/modelkit/commands/serve.py

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
from __future__ import annotations
1717

1818
import logging
19-
import sys
2019
from typing import TYPE_CHECKING
2120

2221
import click
@@ -117,12 +116,10 @@ def serve(
117116
"""
118117
try:
119118
import uvicorn
120-
except ImportError:
121-
click.echo(
122-
"Error: uvicorn is required. Install with: pip install uvicorn[standard]",
123-
err=True,
124-
)
125-
sys.exit(1)
119+
except ImportError as e:
120+
raise click.ClickException(
121+
"uvicorn is required. Install with: pip install uvicorn[standard]"
122+
) from e
126123

127124
if ctx.obj and ctx.obj.get("debug"):
128125
logging.getLogger("modelkit").setLevel(logging.DEBUG)
@@ -135,8 +132,7 @@ def serve(
135132
from ..serve.cli_api import app
136133
from ..serve.cli_api import print_startup_banner as _banner0
137134
except ImportError as e:
138-
click.echo(f"Error: Failed to load serving module: {e}", err=True)
139-
sys.exit(1)
135+
raise click.ClickException(f"Failed to load serving module: {e}") from e
140136
_banner0(host=host, port=port)
141137
uvicorn.run(app, host=host, port=port, reload=auto_reload, log_level="warning")
142138
return
@@ -148,8 +144,7 @@ def serve(
148144
from ..serve.app import create_app
149145
from ..serve.app import print_startup_banner as _banner1
150146
except ImportError as e:
151-
click.echo(f"Error: Failed to load inference serving module: {e}", err=True)
152-
sys.exit(1)
147+
raise click.ClickException(f"Failed to load inference serving module: {e}") from e
153148

154149
mode = "multi" if multi else "single"
155150
inference_app = create_app(

src/winml/modelkit/utils/cli.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,46 @@
3030
OutputFormat: TypeAlias = Literal["text", "json", "table", "compact"]
3131

3232

33+
class ModelLoadError(click.ClickException):
34+
"""Exit code 3: model could not be loaded onto the device/EP.
35+
36+
Use for failures loading a model onto a device/EP, missing accelerators,
37+
or session creation that fails for hardware reasons. The message is printed
38+
verbatim to stderr (no ``Error:`` prefix) so callers control the wording.
39+
"""
40+
41+
exit_code = 3
42+
43+
def show(self, file: Any = None) -> None:
44+
"""Print the message verbatim to stderr (no ``Error:`` prefix)."""
45+
click.echo(self.format_message(), err=True)
46+
47+
48+
class InferenceError(click.ClickException):
49+
"""Exit code 4: inference/prediction failed at runtime.
50+
51+
Use for prediction failures after the model loaded successfully. The
52+
message is printed verbatim to stderr (no ``Error:`` prefix).
53+
"""
54+
55+
exit_code = 4
56+
57+
def show(self, file: Any = None) -> None:
58+
"""Print the message verbatim to stderr (no ``Error:`` prefix)."""
59+
click.echo(self.format_message(), err=True)
60+
61+
62+
class PartialSupportError(click.exceptions.Exit):
63+
"""Exit code 1: a valid negative result, not an error.
64+
65+
Raised silently (no ``Error:`` prefix) so commands can signal an
66+
actionable-but-non-fatal outcome (e.g. analyze: model not fully supported).
67+
"""
68+
69+
def __init__(self) -> None:
70+
super().__init__(1)
71+
72+
3373
# Shared stderr console for security/diagnostic messages emitted from utils.
3474
# Mirrors the module-level ``console = Console()`` pattern used by individual
3575
# command modules, but targets stderr so messages survive ``-q/--quiet``.

0 commit comments

Comments
 (0)