diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a07e9b..08a84db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ### Changed +- Command handlers raise `BambuError` instead of emitting a JSON error and + then aborting. `cli.main` is the sole error-envelope writer + (`write_error_envelope`). Soft statuses (`confirmation_required`, etc.) + still emit their own contracts. + - Tests import the real `paho-mqtt` package instead of stubbing it in `sys.modules`. Audit-named test files are renamed to topic names. diff --git a/bambu_cli/cli.py b/bambu_cli/cli.py index a0f1bb2..022c7da 100644 --- a/bambu_cli/cli.py +++ b/bambu_cli/cli.py @@ -37,7 +37,7 @@ # and the help/workflow smokes plus several tests import it from this module. from .contracts import ErrorEnvelope, Version from .jsonio import json_mode_requested as _json_mode_requested -from .utils import emit_json, emit_json_error +from .utils import emit_json, write_error_envelope def setup_logging(verbose=False, json_mode=False): @@ -191,12 +191,12 @@ def _handle_bambu_error(exc, command_name): # Emit the machine-readable envelope FIRST: stdout must stay parseable even if the # human-readable log line below fails to render. if _json_mode_requested(args) and not utils._JSON_EMITTED: - extra = {} + extra = dict(getattr(exc, "extra", None) or {}) if exc.detail: extra["detail"] = exc.detail if exc.next_command: extra["next_command"] = exc.next_command - emit_json_error( + write_error_envelope( args, command_name, exc.exit_code, @@ -214,7 +214,7 @@ def _handle_interrupt(interrupt_args, command_name): # failure branch) and send the human line to stderr, not stdout. message = "Operation cancelled by user." if _json_mode_requested(interrupt_args) and not utils._JSON_EMITTED: - emit_json_error( + write_error_envelope( interrupt_args, command_name, EXIT_COMMAND_ERROR, @@ -236,14 +236,14 @@ def _handle_interrupt(interrupt_args, command_name): printer_ip = _context.current_settings().printer_ip if printer_ip == "0.0.0.0": message = "Printer IP is not configured. Please run `plate setup` first." - emit_json_error(args, args.cmd or "main", EXIT_CONFIG_ERROR, message, failed_step="config") + write_error_envelope(args, args.cmd or "main", EXIT_CONFIG_ERROR, message, failed_step="config") logger.error(message) sys.exit(EXIT_CONFIG_ERROR) try: socket.getaddrinfo(printer_ip, None) except socket.gaierror: message = f"Invalid printer_ip or hostname in config: {printer_ip}" - emit_json_error(args, args.cmd or "main", EXIT_CONFIG_ERROR, message, failed_step="config") + write_error_envelope(args, args.cmd or "main", EXIT_CONFIG_ERROR, message, failed_step="config") logger.error(message) sys.exit(EXIT_CONFIG_ERROR) @@ -254,7 +254,7 @@ def _handle_interrupt(interrupt_args, command_name): except SystemExit as exc: exit_code = _exit_code_from_system_exit(exc) if exit_code != EXIT_SUCCESS and _json_mode_requested(args) and not utils._JSON_EMITTED: - emit_json_error( + write_error_envelope( args, args.cmd, exit_code, @@ -268,7 +268,7 @@ def _handle_interrupt(interrupt_args, command_name): except Exception as exc: # Envelope first β€” see _safe_log_error: a logging failure must not eat stdout. if _json_mode_requested(args) and not utils._JSON_EMITTED: - emit_json_error( + write_error_envelope( args, args.cmd, EXIT_COMMAND_ERROR, diff --git a/bambu_cli/commands/device.py b/bambu_cli/commands/device.py index 9b0f266..5b11506 100644 --- a/bambu_cli/commands/device.py +++ b/bambu_cli/commands/device.py @@ -8,7 +8,7 @@ from bambu_cli.contracts import Light, Pause, Resume, Stop from bambu_cli.errors import abort from bambu_cli.logging_utils import logger, safe_log_error -from bambu_cli.utils import emit_json, emit_json_error, get_sequence_id +from bambu_cli.utils import emit_json, get_sequence_id def cmd_light(args, ctx=None): @@ -32,9 +32,14 @@ def cmd_light(args, ctx=None): printer = ctx.printer() if not printer.send_command(payload): message = "Failed to send light command." - emit_json_error(args, "light", EXIT_NETWORK_ERROR, message, failed_step="mqtt", action=action, changed=False) safe_log_error(message) - abort("", exit_code=EXIT_NETWORK_ERROR) + abort( + message, + exit_code=EXIT_NETWORK_ERROR, + failed_step="mqtt", + extra={"action": action, "changed": False}, + command="light", + ) logger.info(f"πŸ’‘ Light turned {action}") if bool(_namespace_get(args, "json", False)): emit_json(Light(status="light_changed", command="light", action=action, changed=True)) @@ -60,9 +65,14 @@ def cmd_pause(args, ctx=None): printer = ctx.printer() if not printer.send_command(payload): message = "Failed to send pause command." - emit_json_error(args, "pause", EXIT_NETWORK_ERROR, message, failed_step="mqtt", paused=False) safe_log_error(message) - abort("", exit_code=EXIT_NETWORK_ERROR) + abort( + message, + exit_code=EXIT_NETWORK_ERROR, + failed_step="mqtt", + extra={"paused": False}, + command="pause", + ) logger.info("⏸️ Print paused") if bool(_namespace_get(args, "json", False)): emit_json(Pause(status="paused", command="pause", paused=True)) @@ -88,9 +98,14 @@ def cmd_resume(args, ctx=None): printer = ctx.printer() if not printer.send_command(payload): message = "Failed to send resume command." - emit_json_error(args, "resume", EXIT_NETWORK_ERROR, message, failed_step="mqtt", resumed=False) safe_log_error(message) - abort("", exit_code=EXIT_NETWORK_ERROR) + abort( + message, + exit_code=EXIT_NETWORK_ERROR, + failed_step="mqtt", + extra={"resumed": False}, + command="resume", + ) logger.info("▢️ Print resumed") if bool(_namespace_get(args, "json", False)): emit_json(Resume(status="resumed", command="resume", resumed=True)) @@ -116,9 +131,14 @@ def cmd_stop(args, ctx=None): printer = ctx.printer() if not printer.send_command(payload): message = "Failed to send stop command." - emit_json_error(args, "stop", EXIT_NETWORK_ERROR, message, failed_step="mqtt", stopped=False) safe_log_error(message) - abort("", exit_code=EXIT_NETWORK_ERROR) + abort( + message, + exit_code=EXIT_NETWORK_ERROR, + failed_step="mqtt", + extra={"stopped": False}, + command="stop", + ) logger.info("⏹️ Print stopped") if bool(_namespace_get(args, "json", False)): emit_json(Stop(status="stopped", command="stop", stopped=True)) diff --git a/bambu_cli/commands/files.py b/bambu_cli/commands/files.py index 284b1af..12f65de 100644 --- a/bambu_cli/commands/files.py +++ b/bambu_cli/commands/files.py @@ -15,12 +15,12 @@ ) from bambu_cli.errors import abort from bambu_cli.fsutil import _portable_basename -from bambu_cli.logging_utils import logger, safe_log_error +from bambu_cli.logging_utils import logger from bambu_cli.paths import exception_for_message as _exception_for_message from bambu_cli.paths import expand_path as _expand_path from bambu_cli.paths import path_for_message as _path_for_message from bambu_cli.slicer import _directory_input_message, _is_directory_input -from bambu_cli.utils import emit_json, emit_json_error +from bambu_cli.utils import emit_json def cmd_upload(args, ctx=None): @@ -31,59 +31,64 @@ def cmd_upload(args, ctx=None): filepath = _expand_path(args.file) if filepath.startswith("-"): message = f"Invalid filepath: {_path_for_message(filepath)}" - emit_json_error(args, "upload", EXIT_FILE_ERROR, message, failed_step="validate", file=filepath) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) + abort( + message, + exit_code=EXIT_FILE_ERROR, + failed_step="validate", + extra={"file": filepath}, + ) if not os.path.exists(filepath): message = f"File not found: {_path_for_message(filepath)}" - emit_json_error(args, "upload", EXIT_FILE_ERROR, message, failed_step="validate", file=filepath) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) + abort( + message, + exit_code=EXIT_FILE_ERROR, + failed_step="validate", + extra={"file": filepath}, + ) if _is_directory_input(filepath): message = _directory_input_message(filepath) - emit_json_error(args, "upload", EXIT_FILE_ERROR, message, failed_step="validate", file=filepath) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) + abort( + message, + exit_code=EXIT_FILE_ERROR, + failed_step="validate", + extra={"file": filepath}, + ) filename = _portable_basename(filepath) if _safe_remote_name(filename) is None: message = f"Refusing to upload file with unsafe name: {_name_for_message(filename)!r}" - emit_json_error( - args, "upload", EXIT_FILE_ERROR, message, failed_step="validate", file=filepath, remote_name=filename + abort( + message, + exit_code=EXIT_FILE_ERROR, + failed_step="validate", + extra={"file": filepath, "remote_name": filename}, ) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) if not _is_print_ready_name(filename): message = _print_ready_error_message(filename, "upload") - emit_json_error( - args, "upload", EXIT_FILE_ERROR, message, failed_step="validate", file=filepath, remote_name=filename + abort( + message, + exit_code=EXIT_FILE_ERROR, + failed_step="validate", + extra={"file": filepath, "remote_name": filename}, ) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) try: filesize = os.path.getsize(filepath) except OSError as exc: message = f"Could not read file size for {_path_for_message(filepath)}: {_exception_for_message(exc)}" - emit_json_error( - args, "upload", EXIT_FILE_ERROR, message, failed_step="validate", file=filepath, remote_name=filename + abort( + message, + exit_code=EXIT_FILE_ERROR, + failed_step="validate", + extra={"file": filepath, "remote_name": filename}, ) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) if filesize <= 0: message = f"Refusing to upload empty file: {_path_for_message(filepath)}" - emit_json_error( - args, - "upload", - EXIT_FILE_ERROR, + abort( message, + exit_code=EXIT_FILE_ERROR, failed_step="validate", - file=filepath, - remote_name=filename, - bytes=filesize, + extra={"file": filepath, "remote_name": filename, "bytes": filesize}, ) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) - if getattr(args, "dry_run", False): logger.info(f"πŸ” Dry Run: Validating printer connectivity for {filename}...") printer = get_printer() @@ -108,11 +113,12 @@ def cmd_upload(args, ctx=None): " (a TLS error can mean the camera/FTPS certificate no longer matches a " "configured cert_fingerprint pin β€” verify the printer's certificate)" ) - emit_json_error( - args, "upload", EXIT_NETWORK_ERROR, message, failed_step="dry_run", file=filepath, remote_name=filename + abort( + message, + exit_code=EXIT_NETWORK_ERROR, + failed_step="dry_run", + extra={"file": filepath, "remote_name": filename}, ) - safe_log_error(message) - abort("", exit_code=EXIT_NETWORK_ERROR) logger.info(f" βœ… Local file {_path_for_message(filepath)} exists ({filesize // 1024}KB)") if bool(_namespace_get(args, "json", False)): @@ -193,17 +199,12 @@ def _cb(block): else: # 4 attempts mirrors upload_file.max_retries (3 retries + initial try) message = "Upload failed after 4 attempts." - emit_json_error( - args, - "upload", - EXIT_NETWORK_ERROR, + abort( message, + exit_code=EXIT_NETWORK_ERROR, failed_step="upload", - file=filepath, - remote_name=filename, + extra={"file": filepath, "remote_name": filename}, ) - safe_log_error(f"❌ {message}") - abort("", exit_code=EXIT_NETWORK_ERROR) def cmd_files(args, ctx=None): @@ -238,9 +239,12 @@ def cmd_files(args, ctx=None): logger.info(f" {f}") except Exception as e: message = f"Error listing files: {e}" - emit_json_error(args, "files", EXIT_NETWORK_ERROR, message, failed_step="ftps", files=[]) - safe_log_error(message) - abort("", exit_code=EXIT_NETWORK_ERROR) + abort( + message, + exit_code=EXIT_NETWORK_ERROR, + failed_step="ftps", + extra={"files": []}, + ) def cmd_delete(args, ctx=None): @@ -251,9 +255,12 @@ def cmd_delete(args, ctx=None): filename = str(args.file or "") if _safe_remote_name(filename) is None: message = f"Refusing to delete file with unsafe name: {_name_for_message(filename)!r}" - emit_json_error(args, "delete", EXIT_FILE_ERROR, message, failed_step="validate", file=filename, deleted=False) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) + abort( + message, + exit_code=EXIT_FILE_ERROR, + failed_step="validate", + extra={"file": filename, "deleted": False}, + ) if not args.confirm: logger.warning(f"⚠️ This will DELETE '{filename}' from the printer. Add --confirm to proceed.") if bool(_namespace_get(args, "json", False)): @@ -282,6 +289,9 @@ def cmd_delete(args, ctx=None): raise Exception("Delete operation failed in printer client.") except Exception as e: message = f"Delete failed: {e}" - emit_json_error(args, "delete", EXIT_NETWORK_ERROR, message, failed_step="ftps", file=filename, deleted=False) - safe_log_error(message) - abort("", exit_code=EXIT_NETWORK_ERROR) + abort( + message, + exit_code=EXIT_NETWORK_ERROR, + failed_step="ftps", + extra={"file": filename, "deleted": False}, + ) diff --git a/bambu_cli/commands/gcode.py b/bambu_cli/commands/gcode.py index 62fdc76..bfd6f2c 100644 --- a/bambu_cli/commands/gcode.py +++ b/bambu_cli/commands/gcode.py @@ -8,8 +8,8 @@ from bambu_cli.contracts import Gcode from bambu_cli.download.naming import _has_command_injection_chars from bambu_cli.errors import abort -from bambu_cli.logging_utils import logger, safe_log_error -from bambu_cli.utils import emit_json, emit_json_error, get_sequence_id +from bambu_cli.logging_utils import logger +from bambu_cli.utils import emit_json, get_sequence_id def cmd_gcode(args, ctx=None): @@ -21,10 +21,12 @@ def cmd_gcode(args, ctx=None): # Reject empty/whitespace-only and CR/LF/NUL (shared helper with remote-name # sanitization β€” same command-injection risk on MQTT as on FTP lines). if not gcode.strip() or _has_command_injection_chars(gcode): - message = "Invalid G-code: must be non-empty and must not contain control characters (CR/LF/NUL)." - emit_json_error(args, "gcode", EXIT_COMMAND_ERROR, message, failed_step="validate", gcode=gcode, sent=False) - safe_log_error(message) - abort("", exit_code=EXIT_COMMAND_ERROR) + abort( + "Invalid G-code: must be non-empty and must not contain control characters (CR/LF/NUL).", + exit_code=EXIT_COMMAND_ERROR, + failed_step="validate", + extra={"gcode": gcode, "sent": False}, + ) if not args.confirm: logger.warning("⚠️ This will SEND raw G-code to the printer. Add --confirm to proceed.") @@ -43,10 +45,12 @@ def cmd_gcode(args, ctx=None): payload = json.dumps({"print": {"sequence_id": get_sequence_id(), "command": "gcode_line", "param": gcode}}) printer = ctx.printer() if not printer.send_command(payload): - message = "Failed to send G-code command." - emit_json_error(args, "gcode", EXIT_NETWORK_ERROR, message, failed_step="mqtt", gcode=gcode, sent=False) - safe_log_error(message) - abort("", exit_code=EXIT_NETWORK_ERROR) + abort( + "Failed to send G-code command.", + exit_code=EXIT_NETWORK_ERROR, + failed_step="mqtt", + extra={"gcode": gcode, "sent": False}, + ) logger.info(f"πŸ“‘ Sent: {gcode}") if bool(_namespace_get(args, "json", False)): emit_json(Gcode(status="sent", command="gcode", gcode=gcode, sent=True)) diff --git a/bambu_cli/commands/print_cmd.py b/bambu_cli/commands/print_cmd.py index d790cdd..99ef447 100644 --- a/bambu_cli/commands/print_cmd.py +++ b/bambu_cli/commands/print_cmd.py @@ -1,7 +1,5 @@ """Start a print of a file already on the printer.""" -from bambu_cli import utils -from bambu_cli.argutils import exit_code_from_system_exit as _exit_code_from_system_exit from bambu_cli.argutils import namespace_get as _namespace_get from bambu_cli.constants import EXIT_COMMAND_ERROR, EXIT_FILE_ERROR from bambu_cli.context import RuntimeContext @@ -12,10 +10,10 @@ _print_ready_error_message, _safe_remote_name, ) -from bambu_cli.errors import BambuError, abort -from bambu_cli.job import _last_error_for, _parse_print_options, _print_next_command, generate_print_payload -from bambu_cli.logging_utils import logger, safe_log_error -from bambu_cli.utils import emit_json, emit_json_error +from bambu_cli.errors import abort +from bambu_cli.job import _parse_print_options, _print_next_command, generate_print_payload +from bambu_cli.logging_utils import logger +from bambu_cli.utils import emit_json def cmd_print(args, ctx=None): @@ -27,20 +25,29 @@ def cmd_print(args, ctx=None): if _safe_remote_name(basename) is None: message = f"Refusing to print file with unsafe name: {_name_for_message(basename)!r}" - emit_json_error(args, "print", EXIT_FILE_ERROR, message, failed_step="validate", file=basename) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) + abort( + message, + exit_code=EXIT_FILE_ERROR, + failed_step="validate", + extra={"file": basename}, + ) if not _is_print_ready_name(basename): message = _print_ready_error_message(basename, "print") - emit_json_error(args, "print", EXIT_FILE_ERROR, message, failed_step="validate", file=basename) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) + abort( + message, + exit_code=EXIT_FILE_ERROR, + failed_step="validate", + extra={"file": basename}, + ) ams_mapping, print_option_error = _parse_print_options(args) if print_option_error: - emit_json_error(args, "print", EXIT_COMMAND_ERROR, print_option_error, failed_step="validate", file=basename) - safe_log_error(print_option_error) - abort("", exit_code=EXIT_COMMAND_ERROR) + abort( + print_option_error, + exit_code=EXIT_COMMAND_ERROR, + failed_step="validate", + extra={"file": basename}, + ) if not args.confirm and not dry_run: logger.warning("⚠️ This will START a print. Add --confirm to proceed.") @@ -64,29 +71,11 @@ def cmd_print(args, ctx=None): bed_leveling=not getattr(args, "skip_bed_leveling", False), flow_cali=not getattr(args, "skip_flow_cali", False), ) - try: - from bambu_cli.printer import get_printer - - printer = get_printer() - utils._LAST_ERROR_PAYLOAD = None - from bambu_cli.protocols.mqtt import execute_print_command + from bambu_cli.printer import get_printer + from bambu_cli.protocols.mqtt import execute_print_command - execute_print_command(printer, payload, basename, dry_run=dry_run) - except BambuError as exc: - exit_code = getattr(exc, "exit_code", None) or _exit_code_from_system_exit(exc) - detail = _last_error_for("print") - emit_json_error( - args, - "print", - exit_code, - detail.get("error") if detail else "print failed; see stderr for details", - failed_step="dry_run" if dry_run else "print", - file=basename, - printed=False, - dry_run=bool(dry_run), - **({"print_error": detail} if detail else {}), - ) - raise + printer = get_printer() + execute_print_command(printer, payload, basename, dry_run=dry_run) if bool(_namespace_get(args, "json", False)): emit_json( Print( diff --git a/bambu_cli/commands/snapshot.py b/bambu_cli/commands/snapshot.py index a77071b..61064a1 100644 --- a/bambu_cli/commands/snapshot.py +++ b/bambu_cli/commands/snapshot.py @@ -149,9 +149,14 @@ def _require_localhost_streamer_url(args, streamer_url, outpath): parsed = urlparse(streamer_url) if parsed.scheme not in ("http", "https") or parsed.hostname not in ("localhost", "127.0.0.1", "::1"): message = "Security Error: camera_stream_url must point to localhost." - emit_json_error(args, "snapshot", EXIT_CONFIG_ERROR, message, failed_step="validate", output=outpath) safe_log_error(message) - abort("", exit_code=EXIT_CONFIG_ERROR) + abort( + message, + exit_code=EXIT_CONFIG_ERROR, + failed_step="validate", + extra={"output": outpath}, + command="snapshot", + ) def _write_snapshot_atomic(outpath, data): diff --git a/bambu_cli/context.py b/bambu_cli/context.py index fcb2788..d742cb8 100644 --- a/bambu_cli/context.py +++ b/bambu_cli/context.py @@ -183,12 +183,22 @@ class RuntimeContext: def printer(self) -> BambuPrinter: """Return a cached ``BambuPrinter`` built from ``self.settings``. - Mirrors ``bambu_cli.printer.get_printer()``: empty access_code in - simulation mode, otherwise loaded via ``load_access_code()``. + When this context is the installed process current (the ``cmd_*`` + path via ``for_request``), construction goes through + ``bambu_cli.printer.get_printer()`` so there is one factory β€” tests + that patch ``get_printer`` keep working. A detached context still + builds from ``self.settings`` so library callers do not pick up a + different process-wide context. """ if self._printer is not None: return self._printer + if _current is self: + from bambu_cli.printer import get_printer + + self._printer = get_printer() + return self._printer + from bambu_cli.config import load_access_code from bambu_cli.printer import BambuPrinter from bambu_cli.tlspin import normalize_fingerprint diff --git a/bambu_cli/contracts/__init__.py b/bambu_cli/contracts/__init__.py index 4c21691..356898a 100644 --- a/bambu_cli/contracts/__init__.py +++ b/bambu_cli/contracts/__init__.py @@ -28,6 +28,7 @@ JobError, JobOk, Light, + MigrateAccessCode, OkEnvelope, Pause, Preflight, @@ -66,6 +67,7 @@ "JobError", "JobOk", "Light", + "MigrateAccessCode", "OkEnvelope", "Pause", "Preflight", diff --git a/bambu_cli/contracts/models.py b/bambu_cli/contracts/models.py index f05e983..2d7e9e8 100644 --- a/bambu_cli/contracts/models.py +++ b/bambu_cli/contracts/models.py @@ -443,6 +443,22 @@ class Preflight(Contract): checks: list[PreflightCheck] = spec(required=True, default_factory=list) +@dataclass(frozen=True) +class MigrateAccessCode(Contract): + schema_name: ClassVar[str] = "migrate_access_code" + schema_title: ClassVar[str] = "platecli setup --migrate-access-code envelope" + schema_description: ClassVar[str] = ( + "Result of moving an inline access_code out of config.json into a separate secret file. " + "Never includes the access code value itself." + ) + + status: Literal["migrated", "noop", "error"] + command: Literal["migrate-access-code"] + config_path: str | None = None + access_code_file: str | None = None + reason: str | None = None + + @dataclass(frozen=True) class Doctor(Contract): schema_name: ClassVar[str] = "doctor" diff --git a/bambu_cli/download/downloader.py b/bambu_cli/download/downloader.py index d92b6f1..537c0bf 100644 --- a/bambu_cli/download/downloader.py +++ b/bambu_cli/download/downloader.py @@ -571,6 +571,14 @@ def _cleanup_reserved(): _remove_partial_file(partial_path) _cleanup_reserved() message = f"Download failed: HTTP Error {e.code} ({e.reason})" + if e.code == 404: + logger.info(" The requested file or model does not exist. Check that the URL is correct.") + elif e.code == 403: + logger.info(" Access is forbidden. Printables or the host may be blocking automated requests.") + try: + e.close() + except Exception: + pass emit_json_error( args, "download", @@ -583,16 +591,6 @@ def _cleanup_reserved(): http_status=e.code, path=outpath, ) - safe_log_error(message) - if e.code == 404: - logger.info(" The requested file or model does not exist. Check that the URL is correct.") - elif e.code == 403: - logger.info(" Access is forbidden. Printables or the host may be blocking automated requests.") - try: - e.close() - except Exception: - pass - abort("", exit_code=EXIT_NETWORK_ERROR) except urllib.error.URLError as e: _remove_partial_file(partial_path) _cleanup_reserved() @@ -613,6 +611,7 @@ def _cleanup_reserved(): safe_log_error(message) abort("", exit_code=EXIT_COMMAND_ERROR) message = f"Network error during download: {e}" + logger.info(" Please check your internet connection or verify the domain name resolves correctly.") emit_json_error( args, "download", @@ -624,9 +623,6 @@ def _cleanup_reserved(): download_url=_redact_url_credentials(url), path=outpath, ) - safe_log_error(message) - logger.info(" Please check your internet connection or verify the domain name resolves correctly.") - abort("", exit_code=EXIT_NETWORK_ERROR) except OSError as e: _remove_partial_file(partial_path) _cleanup_reserved() diff --git a/bambu_cli/download/validation.py b/bambu_cli/download/validation.py index 046ba2b..af79322 100644 --- a/bambu_cli/download/validation.py +++ b/bambu_cli/download/validation.py @@ -21,7 +21,6 @@ from bambu_cli.jsonio import redact_url_credentials as _redact_url_credentials from bambu_cli.logging_utils import safe_log_error from bambu_cli.paths import expand_path as _expand_path -from bambu_cli.utils import emit_json_error def _looks_like_url(value): @@ -54,32 +53,37 @@ def _is_http_url(value): def _validate_http_url_or_exit(value): parsed = urlparse(value) if parsed.scheme.lower() not in ("http", "https"): - safe_log_error(f"Invalid URL scheme: {parsed.scheme or 'none'}") - abort("", exit_code=EXIT_COMMAND_ERROR) + message = f"Invalid URL scheme: {parsed.scheme or 'none'}" + safe_log_error(message) + abort(message, exit_code=EXIT_COMMAND_ERROR, failed_step="validate") if not parsed.netloc: - safe_log_error("Invalid URL: missing host") - abort("", exit_code=EXIT_COMMAND_ERROR) + message = "Invalid URL: missing host" + safe_log_error(message) + abort(message, exit_code=EXIT_COMMAND_ERROR, failed_step="validate") if parsed.username is not None or parsed.password is not None: - safe_log_error("Invalid URL: embedded credentials are not supported") - abort("", exit_code=EXIT_COMMAND_ERROR) + message = "Invalid URL: embedded credentials are not supported" + safe_log_error(message) + abort(message, exit_code=EXIT_COMMAND_ERROR, failed_step="validate") def _validate_download_url_or_exit(args, source_url, normalized_source, url, failed_step, label): - """Validate a download URL and emit structured, redacted JSON on failure.""" + """Validate a download URL and attach redacted source fields on failure.""" try: _validate_http_url_or_exit(url) except BambuError as exc: - emit_json_error( - args, - "download", - getattr(exc, "exit_code", getattr(exc, "code", 5)), - f"{label}: {_redact_url_credentials(url)}", - failed_step=failed_step, - source=_redact_url_credentials(source_url), - normalized_source=_redact_url_credentials(normalized_source), - download_url=_redact_url_credentials(url), + extra = { + "source": _redact_url_credentials(source_url), + "normalized_source": _redact_url_credentials(normalized_source), + "download_url": _redact_url_credentials(url), + } + extra.update(exc.extra or {}) + abort( + str(exc), + exit_code=exc.exit_code, + failed_step=failed_step or exc.failed_step, + extra=extra, + command="download", ) - raise def _known_unsupported_download_extension(value): @@ -103,19 +107,17 @@ def _reject_unsupported_download_extension(args, source_url, normalized_source, if not ext: return message = _unsupported_download_message(ext) - emit_json_error( - args, - "download", - EXIT_FILE_ERROR, + abort( message, + exit_code=EXIT_FILE_ERROR, failed_step=failed_step, - source=_redact_url_credentials(source_url), - normalized_source=_redact_url_credentials(normalized_source), - download_url=_redact_url_credentials(url), - extension=ext, + extra={ + "source": _redact_url_credentials(source_url), + "normalized_source": _redact_url_credentials(normalized_source), + "download_url": _redact_url_credentials(url), + "extension": ext, + }, ) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) def _known_unsupported_content_type(content_type): @@ -135,19 +137,17 @@ def _reject_unsupported_content_type(args, source_url, normalized_source, url, c if not media_type: return message = f"Download URL returned unsupported content type '{media_type}', not a model file." - emit_json_error( - args, - "download", - EXIT_FILE_ERROR, + abort( message, + exit_code=EXIT_FILE_ERROR, failed_step="download", - source=_redact_url_credentials(source_url), - normalized_source=_redact_url_credentials(normalized_source), - download_url=_redact_url_credentials(url), - content_type=media_type, + extra={ + "source": _redact_url_credentials(source_url), + "normalized_source": _redact_url_credentials(normalized_source), + "download_url": _redact_url_credentials(url), + "content_type": media_type, + }, ) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) def _max_download_mb_error(args): @@ -164,9 +164,11 @@ def _max_download_mb_error(args): def _validate_max_download_mb_or_exit(args, command="download"): message = _max_download_mb_error(args) if message: - emit_json_error(args, command, EXIT_COMMAND_ERROR, message, failed_step="validate") - safe_log_error(message) - abort("", exit_code=EXIT_COMMAND_ERROR) + abort( + message, + exit_code=EXIT_COMMAND_ERROR, + failed_step="validate", + ) max_download_mb = int(_namespace_get(args, "max_download_mb", DEFAULT_MAX_DOWNLOAD_MB)) return max_download_mb * 1024 * 1024 @@ -179,19 +181,18 @@ def _reject_oversized_download( message = f"Download is too large: {content_length} bytes exceeds the {limit_mb} MB safety limit." else: message = f"Download exceeded the {limit_mb} MB safety limit." - emit_json_error( - args, - "download", - EXIT_FILE_ERROR, + safe_log_error(message) + abort( message, + exit_code=EXIT_FILE_ERROR, failed_step="download", - source=_redact_url_credentials(source_url), - normalized_source=_redact_url_credentials(normalized_source), - download_url=_redact_url_credentials(url), - path=outpath, - received_bytes=received_bytes, - content_length=content_length, - max_download_bytes=max_bytes, + extra={ + "source": _redact_url_credentials(source_url), + "normalized_source": _redact_url_credentials(normalized_source), + "download_url": _redact_url_credentials(url), + "path": outpath, + "received_bytes": received_bytes, + "content_length": content_length, + "max_download_bytes": max_bytes, + }, ) - safe_log_error(message) - abort("", exit_code=EXIT_FILE_ERROR) diff --git a/bambu_cli/errors.py b/bambu_cli/errors.py index 5f87a05..ba8b2a2 100644 --- a/bambu_cli/errors.py +++ b/bambu_cli/errors.py @@ -47,15 +47,42 @@ class BambuError(Exception): exit_code: int = EXIT_COMMAND_ERROR failed_step: str | None = None - def __init__(self, message, *, detail=None, next_command=None, exit_code=None, failed_step=None): + def __init__( + self, + message, + *, + detail=None, + next_command=None, + exit_code=None, + failed_step=None, + extra=None, + ): super().__init__(message) self.detail = detail or {} self.next_command = next_command + self.extra = extra or {} if exit_code is not None: self.exit_code = exit_code if failed_step is not None: self.failed_step = failed_step + def to_error_payload(self, command: str) -> dict: + """Shape job/support reads when a step raises instead of emitting JSON.""" + payload = { + "status": "error", + "command": command, + "exit_code": self.exit_code, + "error": str(self), + } + if self.failed_step: + payload["failed_step"] = self.failed_step + payload.update(self.extra or {}) + if self.detail: + payload["detail"] = self.detail + if self.next_command: + payload["next_command"] = self.next_command + return payload + class ConfigError(BambuError): """Raised when the CLI configuration is missing or invalid.""" @@ -151,13 +178,23 @@ def abort( failed_step=None, detail=None, next_command=None, + extra=None, + command: str | None = None, ) -> NoReturn: - """Raise the appropriate structured error for ``exit_code`` (domain code never calls ``sys.exit``).""" + """Raise the appropriate structured error for ``exit_code`` (domain code never calls ``sys.exit``). + + ``command`` is accepted so leftover ``emit_json_error`` call sites can pass + it through; the exception does not store it β€” callers that need a payload + use ``BambuError.to_error_payload(command)``. + """ + extra = dict(extra or {}) + resolved = message or f"Command failed (exit {exit_code})" cls = _EXIT_TO_EXC.get(exit_code, BambuError) raise cls( - message or f"Command failed (exit {exit_code})", + resolved, exit_code=exit_code, failed_step=failed_step, detail=detail, next_command=next_command, + extra=extra, ) diff --git a/bambu_cli/interactive/session.py b/bambu_cli/interactive/session.py index 84dee30..e85efd4 100644 --- a/bambu_cli/interactive/session.py +++ b/bambu_cli/interactive/session.py @@ -343,14 +343,7 @@ def cmd_go(args: argparse.Namespace, deps: GoDeps | None = None) -> None: json_mode = bool(getattr(args, "json", False)) if json_mode: # Interactive mode has no machine contract; agents already have `job`. - utils.emit_json_error( - args, - "go", - EXIT_COMMAND_ERROR, - _NON_TTY_MESSAGE, - failed_step="parse", - ) - abort(_NON_TTY_MESSAGE, exit_code=EXIT_COMMAND_ERROR, failed_step="parse") + abort(_NON_TTY_MESSAGE, exit_code=EXIT_COMMAND_ERROR, failed_step="parse", command="go") if not sys.stdin.isatty(): abort(_NON_TTY_MESSAGE, exit_code=EXIT_COMMAND_ERROR, failed_step="parse") diff --git a/bambu_cli/job/orchestrate.py b/bambu_cli/job/orchestrate.py index 95d6f25..ed5d013 100644 --- a/bambu_cli/job/orchestrate.py +++ b/bambu_cli/job/orchestrate.py @@ -222,7 +222,7 @@ def _run_job(ctx, args, steps=None): ) ) except BambuError as exc: - detail = _last_error_for("download", ctx) + detail = _last_error_for("download", ctx, exc) _emit_job_failure( args, summary, @@ -423,7 +423,7 @@ def _run_job(ctx, args, steps=None): printable_path = steps.get_slice()(_slice_args_for_job(source_path, args, workdir)) except BambuError as exc: summary["printable_path"] = source_path - detail = _last_error_for("slice", ctx) + detail = _last_error_for("slice", ctx, exc) _emit_job_failure( args, summary, @@ -509,7 +509,7 @@ def _run_job(ctx, args, steps=None): ) ) except BambuError as exc: - detail = _last_error_for("upload", ctx) + detail = _last_error_for("upload", ctx, exc) _emit_job_failure( args, summary, @@ -555,7 +555,7 @@ def _run_job(ctx, args, steps=None): ) ) except BambuError as exc: - detail = _last_error_for("print", ctx) + detail = _last_error_for("print", ctx, exc) summary["next_command"] = ["status", "--json"] summary["recovery_hint"] = ( "Upload succeeded but print start was not confirmed. Check printer status before retrying." diff --git a/bambu_cli/job/support.py b/bambu_cli/job/support.py index 2349766..0e3c994 100644 --- a/bambu_cli/job/support.py +++ b/bambu_cli/job/support.py @@ -91,19 +91,20 @@ def _validate_predicted_remote_name_or_fail(args, summary, remote_name, message_ ) -def _last_error_for(command, ctx=None): - """Return the last-error payload for ``command``, dual-writing it onto - ``ctx.last_error`` when a RuntimeContext is supplied. +def _last_error_for(command, ctx=None, exc=None): + """Return the error payload for ``command`` from ``exc`` or the last abort. - The legacy global (``utils._LAST_ERROR_PAYLOAD``) remains the source of - truth that step implementations write to; ``ctx.last_error`` is a typed - mirror for callers migrating away from the module global. + Prefer the raised ``BambuError`` (the single write path). Fall back to + ``utils._LAST_ERROR_PAYLOAD`` only for steps that still record then raise. """ - payload = utils._LAST_ERROR_PAYLOAD - result = payload if isinstance(payload, dict) and payload.get("command") == command else None + payload = None + if isinstance(exc, BambuError): + payload = exc.to_error_payload(command) + elif isinstance(utils._LAST_ERROR_PAYLOAD, dict) and utils._LAST_ERROR_PAYLOAD.get("command") == command: + payload = utils._LAST_ERROR_PAYLOAD if ctx is not None: - ctx.last_error = result - return result + ctx.last_error = payload + return payload def _dir_is_writable(directory): diff --git a/bambu_cli/setup_cmd/common.py b/bambu_cli/setup_cmd/common.py index 6ec28a1..1281f21 100644 --- a/bambu_cli/setup_cmd/common.py +++ b/bambu_cli/setup_cmd/common.py @@ -372,24 +372,24 @@ def _write_setup_config(config, access_code_file_secret=None): def _setup_summary(config): + from bambu_cli.contracts import Setup + access_code_file = config.get("access_code_file") - payload = { - "status": "configured", - "command": "setup", - "config_path": _display_path(_config_path()), - "printer_ip_configured": bool(config.get("printer_ip")), - "serial_configured": bool(config.get("serial")), - "access_code_storage": "file" if access_code_file else "inline", - "model": config.get("model"), - "nozzle": config.get("nozzle"), - "orca_slicer_configured": bool(config.get("orca_slicer")), - "profiles_dir_configured": bool(config.get("profiles_dir")), - "cert_fingerprint_configured": bool(config.get("cert_fingerprint")), - "insecure_tls": bool(config.get("insecure_tls", False)), - } - if access_code_file: - payload["access_code_file"] = _display_path(access_code_file) - return payload + return Setup( + status="configured", + command="setup", + config_path=_display_path(_config_path()), + printer_ip_configured=bool(config.get("printer_ip")), + serial_configured=bool(config.get("serial")), + access_code_storage="file" if access_code_file else "inline", + model=config.get("model"), + nozzle=config.get("nozzle"), + orca_slicer_configured=bool(config.get("orca_slicer")), + profiles_dir_configured=bool(config.get("profiles_dir")), + cert_fingerprint_configured=bool(config.get("cert_fingerprint")), + insecure_tls=bool(config.get("insecure_tls", False)), + access_code_file=_display_path(access_code_file) if access_code_file else None, + ).to_payload() def _setup_path_details(**paths): diff --git a/bambu_cli/setup_cmd/migrate.py b/bambu_cli/setup_cmd/migrate.py index 8748dce..163747c 100644 --- a/bambu_cli/setup_cmd/migrate.py +++ b/bambu_cli/setup_cmd/migrate.py @@ -164,12 +164,17 @@ def _cmd_migrate_access_code(args): else: logger.error(result["reason"]) - payload = { - "command": "migrate-access-code", - "status": status, - **{k: v for k, v in result.items() if k != "status"}, - } if _namespace_get(args, "json", False): - emit_json(payload) + from bambu_cli.contracts import MigrateAccessCode + + emit_json( + MigrateAccessCode( + status=status, + command="migrate-access-code", + config_path=result.get("config_path"), + access_code_file=result.get("access_code_file"), + reason=result.get("reason"), + ) + ) if status == "error": abort("", exit_code=EXIT_CONFIG_ERROR) diff --git a/bambu_cli/setup_cmd/preflight.py b/bambu_cli/setup_cmd/preflight.py index 20fc640..4ef97c6 100644 --- a/bambu_cli/setup_cmd/preflight.py +++ b/bambu_cli/setup_cmd/preflight.py @@ -332,17 +332,17 @@ def _cmd_preflight(args): status = "warning" if getattr(args, "json", False): - payload = { - "status": status, - "command": "preflight", - "exit_code": exit_code, - "ok": ok, - "errors": error_count, - "warnings": warning_count, - "strict": bool(getattr(args, "strict", False)), - "checks": checks, - } - emit_json(payload) + from bambu_cli.contracts import Preflight + + emit_json( + Preflight(status=status, command="preflight", checks=checks).to_payload( + exit_code=exit_code, + ok=ok, + errors=error_count, + warnings=warning_count, + strict=bool(getattr(args, "strict", False)), + ) + ) else: logger.info("πŸ§ͺ platecli preflight") for check in checks: diff --git a/bambu_cli/setup_cmd/wizard.py b/bambu_cli/setup_cmd/wizard.py index 69ae608..27ce48b 100644 --- a/bambu_cli/setup_cmd/wizard.py +++ b/bambu_cli/setup_cmd/wizard.py @@ -11,7 +11,7 @@ from bambu_cli.config import MODEL_MAPPING, _access_code_value_problem from bambu_cli.constants import EXIT_COMMAND_ERROR, EXIT_CONFIG_ERROR, EXIT_FILE_ERROR, EXIT_NETWORK_ERROR from bambu_cli.errors import abort -from bambu_cli.logging_utils import logger, safe_log_error +from bambu_cli.logging_utils import logger from bambu_cli.paths import display_path as _display_path from bambu_cli.paths import exception_for_message as _exception_for_message from bambu_cli.paths import expand_path as _expand_path @@ -31,7 +31,7 @@ _validate_setup_access_code_file, _write_setup_config, ) -from bambu_cli.utils import emit_json, emit_json_error +from bambu_cli.utils import emit_json def _service_info_address(info): @@ -468,8 +468,10 @@ def _cmd_setup(args): " plate setup --printer-ip 192.168.1.42 --serial 01S00A000000000 " "--access-code-file ~/.bambu_access" ) - emit_json_error(args, "setup", EXIT_CONFIG_ERROR, message, failed_step="validate") - safe_log_error(message) - abort("", exit_code=EXIT_CONFIG_ERROR) + abort( + message, + exit_code=EXIT_CONFIG_ERROR, + failed_step="validate", + ) _cmd_setup_interactive(args) diff --git a/bambu_cli/slicer/cmd.py b/bambu_cli/slicer/cmd.py index 21ff1aa..603d0ea 100644 --- a/bambu_cli/slicer/cmd.py +++ b/bambu_cli/slicer/cmd.py @@ -248,6 +248,12 @@ def cmd_slice( # The envelope needs detected_orca, so resolve it before emitting; the human # log lines come afterwards so a failing handler cannot eat the envelope. detected_orca = detect_orca_slicer() + if detected_orca and detected_orca != settings.orca_slicer: + logger.info( + f'Detected OrcaSlicer at {_display_path(detected_orca)} β€” set "orca_slicer" to this in config.json.' + ) + else: + logger.info("Please update 'orca_slicer' in your config.json or place it in the tools/ directory.") emit_json_error( args, "slice", @@ -258,14 +264,6 @@ def cmd_slice( orca_slicer=settings.orca_slicer, detected_orca_slicer=detected_orca, ) - safe_log_error(message) - if detected_orca and detected_orca != settings.orca_slicer: - logger.info( - f'Detected OrcaSlicer at {_display_path(detected_orca)} β€” set "orca_slicer" to this in config.json.' - ) - else: - logger.info("Please update 'orca_slicer' in your config.json or place it in the tools/ directory.") - abort("", exit_code=EXIT_CONFIG_ERROR) if not os.path.exists(process): compatible_printer = f"{full_model_name} {settings.nozzle_size} nozzle" @@ -299,6 +297,8 @@ def cmd_slice( message = f"Missing {name} profile: {_path_for_message(path)}" # detected_profiles feeds the envelope, so diagnose first, emit, then log. hint, detected_profiles = _profiles_dir_diagnostic(settings.profiles_dir) + if hint: + logger.info(hint) emit_json_error( args, "slice", @@ -311,10 +311,6 @@ def cmd_slice( profiles_dir=settings.profiles_dir, detected_profiles_dir=detected_profiles, ) - safe_log_error(message) - if hint: - logger.info(hint) - abort("", exit_code=EXIT_CONFIG_ERROR) try: tmp_process, tmp_filament = _create_temp_profiles(process, filament, args) diff --git a/bambu_cli/slicer/output.py b/bambu_cli/slicer/output.py index 59cf1c9..d105e4e 100644 --- a/bambu_cli/slicer/output.py +++ b/bambu_cli/slicer/output.py @@ -188,16 +188,6 @@ def _finalize_slice( else: rc = result.returncode if result is not None else -1 message = f"Slicing failed (RC={rc})" - emit_json_error( - args, - "slice", - EXIT_COMMAND_ERROR, - message, - failed_step="slicer", - file=filepath, - output=outpath, - returncode=rc, - ) safe_log_error(message) all_output = "" if result is not None: @@ -213,4 +203,10 @@ def _finalize_slice( if not error_found: logger.info(" Check OrcaSlicer profiles or syntax.") - abort("", exit_code=EXIT_COMMAND_ERROR) + abort( + message, + exit_code=EXIT_COMMAND_ERROR, + failed_step="slicer", + extra={"file": filepath, "output": outpath, "returncode": rc}, + command="slice", + ) diff --git a/bambu_cli/tui/entry.py b/bambu_cli/tui/entry.py index 4a028fd..8855e2d 100644 --- a/bambu_cli/tui/entry.py +++ b/bambu_cli/tui/entry.py @@ -18,7 +18,6 @@ import importlib.util import sys -from bambu_cli import utils from bambu_cli.constants import EXIT_COMMAND_ERROR, EXIT_CONFIG_ERROR from bambu_cli.errors import abort @@ -41,14 +40,7 @@ def cmd_tui(args: argparse.Namespace) -> None: json_mode = bool(getattr(args, "json", False)) if json_mode: # Interactive mode has no machine contract; agents already have `job`. - utils.emit_json_error( - args, - "tui", - EXIT_COMMAND_ERROR, - _NON_TTY_MESSAGE, - failed_step="parse", - ) - abort(_NON_TTY_MESSAGE, exit_code=EXIT_COMMAND_ERROR, failed_step="parse") + abort(_NON_TTY_MESSAGE, exit_code=EXIT_COMMAND_ERROR, failed_step="parse", command="tui") if not sys.stdin.isatty(): abort(_NON_TTY_MESSAGE, exit_code=EXIT_COMMAND_ERROR, failed_step="parse") diff --git a/bambu_cli/utils.py b/bambu_cli/utils.py index c9bc1ec..3dfd9a6 100644 --- a/bambu_cli/utils.py +++ b/bambu_cli/utils.py @@ -150,7 +150,8 @@ def _namespace_get(args, key, default=None): return getattr(args, key, default) -def emit_json_error(args, command, exit_code, error, failed_step=None, **extra): +def write_error_envelope(args, command, exit_code, error, failed_step=None, **extra): + """Write the JSON error envelope. Does not raise β€” ``cli.main`` owns process exit.""" global _JSON_EMITTED _JSON_EMITTED = True global _LAST_ERROR_PAYLOAD @@ -174,6 +175,24 @@ def emit_json_error(args, command, exit_code, error, failed_step=None, **extra): emit_json(payload) +def emit_json_error(args, command, exit_code, error, failed_step=None, **extra): + """Domain failure: log, record extras, then raise. ``cli.main`` emits JSON. + + Kept as a thin wrapper so remaining call sites become a single raise + instead of emit-then-abort. New code should call ``abort`` directly. + """ + from bambu_cli.errors import abort + from bambu_cli.logging_utils import safe_log_error + + extra = dict(extra) + record_error_detail( + command, exit_code, error or f"Command failed (exit {exit_code})", failed_step=failed_step, **extra + ) + if error: + safe_log_error(error) + abort(error, exit_code=exit_code, failed_step=failed_step, extra=extra, command=command) + + def record_error_detail(command, exit_code, error, failed_step=None, **extra): global _LAST_ERROR_PAYLOAD payload = { diff --git a/docs/api.md b/docs/api.md index bdd7a5e..25641ce 100644 --- a/docs/api.md +++ b/docs/api.md @@ -110,6 +110,7 @@ form when looking a code up; the human-readable log line shows both. | [`upload.json`](schemas/upload.json) | `upload` success / `--dry-run` | | [`files.json`](schemas/files.json) | `files` listing | | [`setup.json`](schemas/setup.json) | `setup` summary | +| [`migrate_access_code.json`](schemas/migrate_access_code.json) | `setup --migrate-access-code` | | [`go.json`](schemas/go.json) | `go` error envelope (`--json` always errors) | | [`tui.json`](schemas/tui.json) | `tui` error envelope (`--json` always errors) | diff --git a/docs/schemas/migrate_access_code.json b/docs/schemas/migrate_access_code.json new file mode 100644 index 0000000..73f6f4e --- /dev/null +++ b/docs/schemas/migrate_access_code.json @@ -0,0 +1,35 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://platecli.local/schemas/migrate_access_code.json", + "title": "platecli setup --migrate-access-code envelope", + "type": "object", + "description": "Result of moving an inline access_code out of config.json into a separate secret file. Never includes the access code value itself.", + "required": [ + "status", + "command" + ], + "properties": { + "status": { + "enum": [ + "migrated", + "noop", + "error" + ], + "type": "string" + }, + "command": { + "const": "migrate-access-code", + "type": "string" + }, + "config_path": { + "type": "string" + }, + "access_code_file": { + "type": "string" + }, + "reason": { + "type": "string" + } + }, + "additionalProperties": true +} diff --git a/tests/contracts/test_contract_models.py b/tests/contracts/test_contract_models.py index c6628e0..b14a8cd 100644 --- a/tests/contracts/test_contract_models.py +++ b/tests/contracts/test_contract_models.py @@ -210,6 +210,12 @@ def test_committed_schemas_match_the_contracts(): serial_configured=True, access_code_storage="file", ), + contracts.MigrateAccessCode( + status="migrated", + command="migrate-access-code", + config_path="/tmp/config.json", + access_code_file="/tmp/access_code", + ), contracts.ConfigCmd(status="ok", command="config", action="show"), contracts.Preflight( status="ok", diff --git a/tests/contracts/test_schema_validation.py b/tests/contracts/test_schema_validation.py index f12511d..d705e23 100644 --- a/tests/contracts/test_schema_validation.py +++ b/tests/contracts/test_schema_validation.py @@ -106,7 +106,7 @@ def _reset(): # Schemas that are not tied to one subcommand: the shared envelopes plus # `--version`, which is a global flag rather than a subcommand. -_SHARED_SCHEMAS = {"error_envelope.json", "ok_envelope.json", "version.json"} +_SHARED_SCHEMAS = {"error_envelope.json", "ok_envelope.json", "version.json", "migrate_access_code.json"} def _parser_subcommands(): """Same derivation idiom as scripts/cli_help_smoke.py, deliberately.""" diff --git a/tests/test_cli_envelopes.py b/tests/test_cli_envelopes.py index 4a01af1..37db6cd 100644 --- a/tests/test_cli_envelopes.py +++ b/tests/test_cli_envelopes.py @@ -122,9 +122,8 @@ def test_direct_write_oserror_becomes_file_error(self, _mock_logger): class TestUploadDryRunReason(unittest.TestCase): - @patch("bambu_cli.commands.files.safe_log_error") @patch("bambu_cli.printer.get_printer") - def test_dry_run_surfaces_ssl_pin_reason(self, mock_get_printer, mock_safe_log): + def test_dry_run_surfaces_ssl_pin_reason(self, mock_get_printer): from bambu_cli.commands import files from bambu_cli.constants import EXIT_NETWORK_ERROR @@ -142,16 +141,13 @@ def test_dry_run_surfaces_ssl_pin_reason(self, mock_get_printer, mock_safe_log): args = argparse.Namespace(file=fpath, dry_run=True, json=True, verbose=False) - buf = io.StringIO() - with settings_ctx(simulation=False), redirect_stdout(buf): + with settings_ctx(simulation=False): with self.assertRaises(BambuError) as cm: files.cmd_upload(args) self.assertEqual(cm.exception.exit_code, EXIT_NETWORK_ERROR) - payload = json.loads(buf.getvalue()) # The real cause (SSL/fingerprint) must appear β€” not the old fixed # "Could not reach printer." string with no detail. - self.assertIn("fingerprint", payload["error"].lower()) - self.assertNotEqual(payload["error"], "Dry run failed: Could not reach printer.") + self.assertIn("fingerprint", str(cm.exception).lower()) # --------------------------------------------------------------------------- diff --git a/tests/test_cmd_files.py b/tests/test_cmd_files.py index 5557efb..383419c 100644 --- a/tests/test_cmd_files.py +++ b/tests/test_cmd_files.py @@ -69,12 +69,12 @@ def test_cmd_files_error(self, mock_exit, mock_logger, mock_get_printer): self._printer_with_ftp(mock_get_printer, mock_get_ftp) mock_exit.side_effect = SystemExit(2) - with self.assertRaises((SystemExit, BambuError)): + with self.assertRaises(BambuError) as cm: cmd_files(args) mock_get_ftp.assert_called_once() mock_ftp.nlst.assert_called_once_with("/model/") - mock_logger.error.assert_called_with("Error listing files: Failed to list files via printer API") + self.assertIn("Failed to list files", str(cm.exception)) @patch("bambu_cli.printer.get_printer") @patch("bambu_cli.logging_utils._BACKEND") @@ -88,11 +88,11 @@ def test_cmd_files_get_ftp_error(self, mock_exit, mock_logger, mock_get_printer) self._printer_with_ftp(mock_get_printer, mock_get_ftp) mock_exit.side_effect = SystemExit(2) - with self.assertRaises((SystemExit, BambuError)): + with self.assertRaises(BambuError) as cm: cmd_files(args) mock_get_ftp.assert_called_once() - mock_logger.error.assert_called_with("Error listing files: Failed to list files via printer API") + self.assertIn("Failed to list files", str(cm.exception)) class TestBambuCmdDelete(unittest.TestCase): @@ -158,9 +158,9 @@ def test_cmd_delete_error(self, mock_exit, mock_logger, mock_get_printer): mock_get_printer.return_value = printer mock_exit.side_effect = SystemExit(2) - with self.assertRaises((SystemExit, BambuError)): + with self.assertRaises(BambuError) as cm: cmd_delete(args) mock_get_ftp.assert_called_once() mock_ftp.delete.assert_called_once_with("/model/test.3mf") - mock_logger.error.assert_called_with("Delete failed: Delete operation failed in printer client.") + self.assertIn("Delete", str(cm.exception)) diff --git a/tests/test_cmd_upload.py b/tests/test_cmd_upload.py index efaa930..70e200c 100644 --- a/tests/test_cmd_upload.py +++ b/tests/test_cmd_upload.py @@ -17,7 +17,7 @@ def test_cmd_upload_invalid_filepath(self, mock_exit, mock_logger): with self.assertRaises((SystemExit, BambuError)) as cm: cmd_upload(args) self.assertEqual(getattr(cm.exception, "exit_code", getattr(cm.exception, "code", None)), 3) - mock_logger.error.assert_called_with("Invalid filepath: -invalid.gcode") + self.assertIn("Invalid filepath", str(cm.exception)) @patch("os.path.exists") @patch("bambu_cli.logging_utils._BACKEND") @@ -32,7 +32,7 @@ def test_cmd_upload_file_not_found(self, mock_exit, mock_logger, mock_exists): with self.assertRaises((SystemExit, BambuError)) as cm: cmd_upload(args) self.assertEqual(getattr(cm.exception, "exit_code", getattr(cm.exception, "code", None)), 3) - mock_logger.error.assert_called_with("File not found: missing.gcode") + self.assertIn("File not found", str(cm.exception)) @patch("os.path.exists") @patch("os.path.getsize") @@ -84,7 +84,7 @@ def test_cmd_upload_dry_run_fail(self, mock_exit, mock_logger, mock_get_printer, # The dry-run now surfaces the real cause instead of a fixed, misleading # "Could not reach printer." (a cert-pin mismatch must be distinguishable # from an off printer) β€” see fix/audit-cli-json-camera. - mock_logger.error.assert_called_with("Dry run failed: could not reach printer: FTP Error") + self.assertIn("FTP Error", str(cm.exception)) @patch("os.path.exists") @patch("os.path.getsize") @@ -162,7 +162,7 @@ def test_cmd_upload_max_retries_exhausted( cmd_upload(args) self.assertEqual(getattr(cm.exception, "exit_code", getattr(cm.exception, "code", None)), 2) - mock_logger.error.assert_called_with("❌ Upload failed after 4 attempts.") + self.assertIn("Upload failed", str(cm.exception)) class TestBambuUploadRetry(unittest.TestCase): @patch("bambu_cli.printer.get_printer") diff --git a/tests/test_download_validation_boundary.py b/tests/test_download_validation_boundary.py index 8e810d7..0d84009 100644 --- a/tests/test_download_validation_boundary.py +++ b/tests/test_download_validation_boundary.py @@ -47,9 +47,6 @@ def _reset_json_state(): def _args(**kw): return Namespace(json=True, **kw) -def _payload(capsys): - return json.loads(capsys.readouterr().out) - # --- unsupported source extension ------------------------------------------- @pytest.mark.parametrize("value", ["archive.rar", "notes.pdf", "/path/to/render.png", "a.tar", "b.7z"]) @@ -84,8 +81,9 @@ def test_reject_unsupported_extension_aborts_with_file_error(capsys): with pytest.raises(BambuError) as excinfo: V._reject_unsupported_download_extension(_args(), URL, None, URL, "archive.rar") assert getattr(excinfo.value, "exit_code", None) == EXIT_FILE_ERROR + assert capsys.readouterr().out == "" - payload = _payload(capsys) + payload = excinfo.value.to_error_payload("download") assert payload["status"] == "error" assert payload["command"] == "download" assert payload["failed_step"] == "validate" @@ -93,9 +91,10 @@ def test_reject_unsupported_extension_aborts_with_file_error(capsys): def test_reject_unsupported_extension_honours_the_caller_step(capsys): # The same refusal happens mid-download after a redirect; the step must say so. - with pytest.raises(BambuError): + with pytest.raises(BambuError) as excinfo: V._reject_unsupported_download_extension(_args(), URL, None, URL, "archive.rar", failed_step="download") - assert _payload(capsys)["failed_step"] == "download" + assert capsys.readouterr().out == "" + assert excinfo.value.to_error_payload("download")["failed_step"] == "download" def test_reject_unsupported_extension_is_a_no_op_for_supported_types(capsys): V._reject_unsupported_download_extension(_args(), URL, None, URL, "model.stl") @@ -107,9 +106,10 @@ def test_rejection_redacts_credentials_in_the_url(capsys): # tests/privacy_smoke.py rejects. Same convention as the sibling tests in # test_job.py and test_mqtt_print_and_setup.py. creds = "http://user@127.0.0.1/archive.rar" - with pytest.raises(BambuError): + with pytest.raises(BambuError) as excinfo: V._reject_unsupported_download_extension(_args(), creds, None, creds, "archive.rar") - emitted = capsys.readouterr().out + assert capsys.readouterr().out == "" + emitted = json.dumps(excinfo.value.to_error_payload("download")) assert "user@" not in emitted, "userinfo leaked into the error envelope" # --- unsupported content type ------------------------------------------------ @@ -144,8 +144,9 @@ def test_reject_unsupported_content_type_reports_the_download_step(capsys): with pytest.raises(BambuError) as excinfo: V._reject_unsupported_content_type(_args(), URL, None, URL, "image/png") assert getattr(excinfo.value, "exit_code", None) == EXIT_FILE_ERROR + assert capsys.readouterr().out == "" - payload = _payload(capsys) + payload = excinfo.value.to_error_payload("download") # It failed after the request went out, so this is `download`, not `validate`. assert payload["failed_step"] == "download" assert payload["content_type"] == "image/png" @@ -162,9 +163,9 @@ def test_failure_detail_is_recorded_for_non_json_callers(capsys): Without --json nothing is printed, but the detail must still be captured or a pipeline failure loses the reason it failed. """ - with pytest.raises(BambuError): + with pytest.raises(BambuError) as excinfo: V._reject_unsupported_download_extension(Namespace(json=False), URL, None, URL, "archive.rar") assert capsys.readouterr().out == "" - assert utils._LAST_ERROR_PAYLOAD is not None - assert utils._LAST_ERROR_PAYLOAD["failed_step"] == "validate" - assert utils._LAST_ERROR_PAYLOAD["extension"] == ".rar" + payload = excinfo.value.to_error_payload("download") + assert payload["failed_step"] == "validate" + assert payload["extension"] == ".rar" diff --git a/tests/test_interactive_session.py b/tests/test_interactive_session.py index 30bcda9..0037d90 100644 --- a/tests/test_interactive_session.py +++ b/tests/test_interactive_session.py @@ -471,9 +471,9 @@ def test_json_mode_emits_error_envelope_and_exits_5(tmp_path, capsys): with pytest.raises(BambuError) as ei: cmd_go(_args(json=True)) assert ei.value.exit_code == 5 - import json as _json - - payload = _json.loads(capsys.readouterr().out) + assert ei.value.failed_step == "parse" + assert capsys.readouterr().out == "" + payload = ei.value.to_error_payload("go") assert payload["status"] == "error" assert payload["command"] == "go" assert payload["exit_code"] == 5 diff --git a/tests/test_json_envelope_ordering.py b/tests/test_json_envelope_ordering.py index 03b07dc..11974f4 100644 --- a/tests/test_json_envelope_ordering.py +++ b/tests/test_json_envelope_ordering.py @@ -105,20 +105,12 @@ def _setup_headless(args): ], ) def test_json_envelope_survives_logger_failure(label, invoke, command, failed_step, capsys): - from bambu_cli import utils - + """Domain helpers raise; they must not emit JSON themselves (cli.main does).""" args = Namespace(json=True) - broken = _broken_logger() - utils._JSON_EMITTED = False - with patch("bambu_cli.logging_utils._BACKEND", broken), pytest.raises(BambuError): + with pytest.raises(BambuError) as cm: invoke(args) - - payload = _envelope(capsys) - assert payload["status"] == "error", label - assert payload["command"] == command, label - assert payload["failed_step"] == failed_step, label - # The handler really did explode, and safe_log_error absorbed it. - assert broken.error.called, label + assert cm.value.failed_step == failed_step, (label, cm.value.failed_step) + assert capsys.readouterr().out == "" def test_safe_log_error_falls_back_to_stderr(capsys): diff --git a/tests/test_mqtt_print_and_setup.py b/tests/test_mqtt_print_and_setup.py index 04c538e..af9e6d2 100644 --- a/tests/test_mqtt_print_and_setup.py +++ b/tests/test_mqtt_print_and_setup.py @@ -329,11 +329,13 @@ def test_json_envelope_survives_logger_failure(cmd_name, args, capsys): ctx = MagicMock() ctx.printer.return_value = printer fr.return_value = ctx - with pytest.raises(BambuError): + with pytest.raises(BambuError) as ei: getattr(commands_mod, cmd_name)(args) - # The envelope reached stdout even though the log handler exploded. - payload = json.loads(capsys.readouterr().out) + # Domain raises; cli.main writes the envelope. The exception must still + # carry the contract fields, and the exploding handler must not leak. + assert capsys.readouterr().out == "" + payload = ei.value.to_error_payload(cmd_name.removeprefix("cmd_")) assert payload["status"] == "error" assert payload["command"] == cmd_name.removeprefix("cmd_") assert payload["failed_step"] == "mqtt" diff --git a/tests/test_platform_paths.py b/tests/test_platform_paths.py index eb5ec46..8a293b0 100644 --- a/tests/test_platform_paths.py +++ b/tests/test_platform_paths.py @@ -83,10 +83,13 @@ def test_preflight_permission_check(tmp_path): def test_common_setup_json_error(capsys): args = Namespace(json=True) - common_mod._setup_json_error(args, "boom", foo=1) - data = json.loads(capsys.readouterr().out) + with pytest.raises(BambuError) as ei: + common_mod._setup_json_error(args, "boom", foo=1) + assert capsys.readouterr().out == "" + data = ei.value.to_error_payload("setup") assert data["status"] == "error" assert data["error"] == "boom" + assert data["foo"] == 1 def test_ftps_connection_error_path_cleanup(): ftp = ftps_mod.ImplicitFTPS() diff --git a/tests/test_slice_stub_integration.py b/tests/test_slice_stub_integration.py index 70346d2..1b52337 100644 --- a/tests/test_slice_stub_integration.py +++ b/tests/test_slice_stub_integration.py @@ -210,9 +210,10 @@ def test_nonzero_exit_failure_aborts(orca_env): def test_nonzero_exit_failure_json_envelope(orca_env, capsys): args = _slice_args(orca_env.model, orca_env.outdir, json=True) - with pytest.raises(BambuError): + with pytest.raises(BambuError) as ei: orca_env("fail", args=args) - payload = _last_json_object(capsys.readouterr().out) + assert capsys.readouterr().out == "" + payload = ei.value.to_error_payload("slice") assert payload["status"] == "error" assert payload["command"] == "slice" assert payload["failed_step"] == "slicer" @@ -269,9 +270,10 @@ def test_stale_output_json_envelope_reports_slicer_failure(orca_env, capsys): old = os.stat(orca_env.outpath) os.utime(orca_env.outpath, ns=(old.st_atime_ns - 10_000_000_000, old.st_mtime_ns - 10_000_000_000)) args = _slice_args(orca_env.model, orca_env.outdir, json=True) - with pytest.raises(BambuError): + with pytest.raises(BambuError) as ei: orca_env("benign_gl_no_write", args=args) - payload = _last_json_object(capsys.readouterr().out) + assert capsys.readouterr().out == "" + payload = ei.value.to_error_payload("slice") assert payload["status"] == "error" assert payload["failed_step"] == "slicer" diff --git a/tests/test_tui_entry.py b/tests/test_tui_entry.py index df80fae..1525063 100644 --- a/tests/test_tui_entry.py +++ b/tests/test_tui_entry.py @@ -10,7 +10,6 @@ import argparse import importlib.util -import json as _json import sys import pytest @@ -43,7 +42,9 @@ def test_json_mode_emits_error_envelope_and_exits_5(capsys): with pytest.raises(BambuError) as ei: cmd_tui(_args(json=True)) assert ei.value.exit_code == 5 - payload = _json.loads(capsys.readouterr().out) + assert ei.value.failed_step == "parse" + assert capsys.readouterr().out == "" + payload = ei.value.to_error_payload("tui") assert payload["status"] == "error" assert payload["command"] == "tui" assert payload["exit_code"] == 5