From bbdb8e0ecf06a405b756c980b30653b06fedd62c Mon Sep 17 00:00:00 2001 From: calvinleng-science Date: Thu, 17 Sep 2026 14:56:43 -0700 Subject: [PATCH 1/6] Improve CLI error reporting Render expected RPC and semantic failures without raw transport internals or unrelated usage text, while preserving the existing Device client behavior by opting in from CLI callers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- synapse/cli/__main__.py | 41 ++++++----- synapse/cli/deploy.py | 12 ++- synapse/cli/errors.py | 58 +++++++++++++++ synapse/cli/files.py | 9 ++- synapse/cli/query.py | 43 ++++++++--- synapse/cli/rpc.py | 62 ++++++++++------ synapse/cli/settings.py | 14 +++- synapse/cli/streaming.py | 37 ++++++---- synapse/client/device.py | 44 +++++++---- synapse/tests/cli/test_errors.py | 123 +++++++++++++++++++++++++++++++ 10 files changed, 346 insertions(+), 97 deletions(-) create mode 100644 synapse/cli/errors.py create mode 100644 synapse/tests/cli/test_errors.py diff --git a/synapse/cli/__main__.py b/synapse/cli/__main__.py index ccd89f1e..dfd1db0d 100755 --- a/synapse/cli/__main__.py +++ b/synapse/cli/__main__.py @@ -21,6 +21,7 @@ taps, settings, ) +from synapse.cli.errors import run_action from synapse.utils.discover import find_device_by_name @@ -47,7 +48,10 @@ def setup_device_uri(args): def main(): - logging.basicConfig(level=logging.INFO, handlers=[RichHandler()]) + logging.basicConfig( + level=logging.INFO, + handlers=[RichHandler(show_path=False)], + ) parser = argparse.ArgumentParser( description="Synapse Device Manager", formatter_class=lambda prog: argparse.HelpFormatter(prog, width=124), @@ -85,27 +89,24 @@ def main(): deploy_model.add_commands(subparsers) args = peripherals.parse_args_with_passthrough(parser) - # If we need to setup the device URI, do that now - args = setup_device_uri(args) - if not args: - return + def run_parsed_command(): + # If we need to setup the device URI, do that now + resolved_args = setup_device_uri(args) + if not resolved_args: + return False + + if hasattr(resolved_args, "func"): + return resolved_args.func(resolved_args) - try: - if hasattr(args, "func"): - args.func(args) - else: - parser.print_help() - except Exception as e: - console = Console() - console.log(f"[bold red] Uncaught error during function. Why: {e}") parser.print_help() - except KeyboardInterrupt: - print("User cancelled request") + return None + + return run_action( + run_parsed_command, + console=Console(stderr=True), + verbose=args.verbose, + ) if __name__ == "__main__": - try: - main() - except Exception as e: - print(f"Uncaught error in CLI. Why: {e}") - sys.exit(1) + sys.exit(main()) diff --git a/synapse/cli/deploy.py b/synapse/cli/deploy.py index 84b3f36d..c2acfc30 100644 --- a/synapse/cli/deploy.py +++ b/synapse/cli/deploy.py @@ -4,6 +4,7 @@ from rich.console import Console, Group from rich.live import Live from rich.panel import Panel +from synapse.cli.errors import error_message from rich.spinner import Spinner from rich.text import Text @@ -81,7 +82,7 @@ def deploy_package(ip_address, deb_package_path): package_filename = os.path.basename(deb_package_path) console.clear_live() - device = syn.Device(ip_address, False) + device = syn.Device(ip_address, False, raise_rpc_errors=True) metadata = create_metadata(deb_package_path, console) console.print( f"[bold green]Deploying:[/bold green] [cyan]{package_filename}[/cyan]" @@ -178,7 +179,9 @@ def chunk_generator(): break # Add the error message at the bottom - display_items.append(f"[bold red]Error: {str(e)}[/bold red]") + display_items.append( + f"[bold red]Error:[/bold red] {error_message(e, device.verbose)}" + ) # Update the panel with progress and error response_panel.renderable = Group(*display_items) @@ -196,7 +199,10 @@ def chunk_generator(): display_items.append(f"[green]✓[/green] Step {i + 1}: {resp}") # Add the error message - display_items.append(f"[bold red]Error during setup: {str(e)}[/bold red]") + display_items.append( + "[bold red]Error:[/bold red] " + f"Deployment setup failed: {error_message(e, device.verbose)}" + ) # Update the panel with progress and error response_panel.renderable = Group(*display_items) diff --git a/synapse/cli/errors.py b/synapse/cli/errors.py new file mode 100644 index 00000000..e6285176 --- /dev/null +++ b/synapse/cli/errors.py @@ -0,0 +1,58 @@ +from __future__ import annotations + +from collections.abc import Callable +from typing import Optional + +import grpc +from rich.console import Console +from rich.text import Text + + +def error_message(error: BaseException, verbose: bool = False) -> str: + """Return a concise user-facing message for an exception.""" + if not isinstance(error, grpc.RpcError): + return str(error) or error.__class__.__name__ + + details = error.details() + message = details or "The device did not provide an error message." + if not verbose: + return message + + code = error.code() + code_name = getattr(code, "name", str(code)) if code is not None else "UNKNOWN" + return f"gRPC {code_name}: {message}" + + +def print_error( + console: Console, + error: BaseException, + *, + context: Optional[str] = None, + verbose: bool = False, +) -> None: + message = error_message(error, verbose=verbose) + prefix = f"{context}: " if context else "" + console.print("[bold red]Error:[/bold red]", Text(f"{prefix}{message}")) + + +def run_action( + action: Callable[[], object], + *, + console: Console, + verbose: bool = False, +) -> int: + """Run a parsed CLI action and translate runtime failures to exit codes.""" + try: + result = action() + return 1 if result is False else 0 + except KeyboardInterrupt: + console.print("[yellow]Operation cancelled.[/yellow]") + return 130 + except grpc.RpcError as error: + print_error(console, error, verbose=verbose) + return 1 + except Exception as error: + print_error(console, error) + if verbose: + console.print_exception(show_locals=False) + return 1 diff --git a/synapse/cli/files.py b/synapse/cli/files.py index ab3844cc..cb5bc888 100644 --- a/synapse/cli/files.py +++ b/synapse/cli/files.py @@ -12,6 +12,7 @@ from rich.prompt import Confirm from synapse import Device +from synapse.cli.errors import print_error import synapse.client.sftp as sftp from synapse.utils.file import format_mode, format_time, filesize_binary @@ -105,7 +106,7 @@ def ls(args): file_attr = sftp_conn.listdir_attr(args.path) print_file_list(file_attr, console) except Exception as e: - console.print(f"[bold red]Failed to list directory:[/bold red] {e}") + print_error(console, e, context="Failed to list directory") sftp.close_sftp(ssh, sftp_conn) @@ -155,7 +156,7 @@ def setup_connection( forget_password: bool, console: Console, ) -> Optional[tuple[paramiko.SSHClient, paramiko.SFTPClient]]: - dev_name = Device(uri).get_name() + dev_name = Device(uri, raise_rpc_errors=True).get_name() password = find_password( dev_name, env_file ) # Check if password is provided or stored in env file @@ -311,7 +312,7 @@ def update_progress(transferred: int, total: int): sftp_conn.get(remote_path, local_path, callback=update_progress) except paramiko.SFTPError as e: - console.print(f"[bold red]Failed to download file:[/bold red] {e}") + print_error(console, e, context="Failed to download file") return @@ -343,7 +344,7 @@ def remove_file( ): sftp_conn.remove(remote_path) except Exception as e: - console.print(f"[bold red]Failed to remove file:[/bold red] {e}") + print_error(console, e, context="Failed to remove file") return console.print(f"[bold green]File removed:[/bold green] [blue]{remote_path}") diff --git a/synapse/cli/query.py b/synapse/cli/query.py index 27cd3d2d..25edc254 100644 --- a/synapse/cli/query.py +++ b/synapse/cli/query.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 import asyncio +import grpc from threading import Thread import time import sys @@ -20,6 +21,8 @@ from rich.live import Live from rich.panel import Panel +from synapse.cli.errors import print_error + class StreamingQueryClient: def __init__(self, uri, verbose=False): @@ -27,7 +30,7 @@ def __init__(self, uri, verbose=False): self.verbose = verbose self.console = Console() - self.device = syn.Device(self.uri, self.verbose) + self.device = syn.Device(self.uri, self.verbose, raise_rpc_errors=True) if self.verbose: info = self.device.info() self.console.log(info) @@ -45,12 +48,20 @@ def close(self): def tail_logs_background(self): self.last_log_line = "" - for log in self.device.tail_logs(): - if self.last_log_line != log.message: - self.last_log_line = log.message - self.new_log_event.set() - if self.log_stop_event.is_set(): - break + try: + for log in self.device.tail_logs(): + if self.last_log_line != log.message: + self.last_log_line = log.message + self.new_log_event.set() + if self.log_stop_event.is_set(): + break + except grpc.RpcError as error: + print_error( + self.console, + error, + context="Log stream failed", + verbose=self.verbose, + ) def stream_query(self, request): query_type = request.request.query_type @@ -63,7 +74,12 @@ def stream_query(self, request): self.console.log(f"[bold red]Unknown stream request: {query_type}") return False except Exception as e: - self.console.log(f"[bold red] Uncaught exception during stream: {e}") + print_error( + self.console, + e, + context="Streaming query failed", + verbose=self.verbose, + ) return False except KeyboardInterrupt: self.console.log("[yellow] Operation cancelled by user") @@ -103,7 +119,7 @@ def update_status(): if response.code != 0 or not response.self_test: self.console.log( - f"[bold red] Failed self test, why: {response.message}" + f"[bold red]Self test failed:[/bold red] {response.message}" ) return False @@ -210,7 +226,7 @@ def update_progress(): failed_ids = [m.electrode_id for m in failed_batch] progress.console.log( - f"Failed to measure impedance for {failed_ids}, why: {response.message}" + f"Failed to measure impedance for {failed_ids}: {response.message}" ) for sample in failed_batch: progress.console.log( @@ -317,5 +333,10 @@ def load_config_from_file(path_to_config): print("Failed to stream query for device") sys.exit(1) except Exception as e: - print(f"Failed to stream query. Why: {e}") + print_error( + Console(stderr=True), + e, + context="Streaming query failed", + verbose=args.verbose, + ) sys.exit(1) diff --git a/synapse/cli/rpc.py b/synapse/cli/rpc.py index 8ddfb739..c17a9818 100644 --- a/synapse/cli/rpc.py +++ b/synapse/cli/rpc.py @@ -14,6 +14,7 @@ from synapse.cli.query import StreamingQueryClient from synapse.cli import impedance_csv +from synapse.cli.errors import print_error from synapse.utils.log import log_entry_to_str from synapse.cli.device_info_display import DeviceInfoDisplay from synapse.utils.proto import load_device_config @@ -100,7 +101,7 @@ def add_commands(subparsers): def info(args): - device = syn.Device(args.uri, args.verbose) + device = syn.Device(args.uri, args.verbose, raise_rpc_errors=True) display = DeviceInfoDisplay() display.summary(device) @@ -116,7 +117,7 @@ def load_query_request(path_to_config): console.print(f"[red]Failed to open {path_to_config}: File not found[/red]") return None except Exception as e: - console.print(f"[red]Failed to parse query file: {str(e)}[/red]") + print_error(console, e, context="Failed to parse query file") return None console = Console() @@ -128,7 +129,12 @@ def load_query_request(path_to_config): try: return client.stream_query(StreamQueryRequest(request=query_proto)) except Exception as e: - console.print(f"[red]Error streaming query: {str(e)}[/red]") + print_error( + console, + e, + context="Streaming query failed", + verbose=args.verbose, + ) return False if Path(args.query_file).suffix != ".json": @@ -141,7 +147,7 @@ def load_query_request(path_to_config): console.print("Running query:") console.print(query_proto) - device = syn.Device(args.uri, args.verbose) + device = syn.Device(args.uri, args.verbose, raise_rpc_errors=True) # Resolve the peripheral name before running the query, matching the # streaming path: if the probe un-enumerates as a result of the query @@ -170,11 +176,13 @@ def load_query_request(path_to_config): f"[green]Impedance measurements saved to {filename}[/green]" ) except IOError as e: - console.print( - f"[red]Error writing impedance measurements: {str(e)}[/red]" + print_error( + console, + e, + context="Failed to write impedance measurements", ) except Exception as e: - console.print(f"[red]Error executing query: {str(e)}[/red]") + print_error(console, e, context="Query failed", verbose=args.verbose) return False @@ -203,12 +211,14 @@ def start(args): try: config_obj = load_device_config(cfg_path, console) except Exception as e: - console.print( - f"[bold red]Failed to parse configuration file[/bold red]: {e}" + print_error( + console, + e, + context="Failed to parse configuration file", ) return - device = syn.Device(args.uri, args.verbose) + device = syn.Device(args.uri, args.verbose, raise_rpc_errors=True) device_name = device.get_name() @@ -217,8 +227,8 @@ def start(args): with console.status("Configuring device...", spinner="bouncingBall"): cfg_ret = device.configure_with_status(config_obj) if cfg_ret is None: - console.print("[bold red]Internal error configuring device") - return + console.print("[bold red]Failed to configure device[/bold red]") + return False if cfg_ret.code != StatusCode.kOk: console.print( f"[bold red]Error configuring device[/bold red]\nResponse from {device_name}:\n{cfg_ret.message}" @@ -229,8 +239,8 @@ def start(args): with console.status("Starting device...", spinner="bouncingBall"): start_ret = device.start_with_status() if start_ret is None: - console.print("[bold red]Internal error starting device") - return + console.print("[bold red]Failed to start device[/bold red]") + return False if start_ret.code != StatusCode.kOk: console.print( f"[bold red]Error starting device[/bold red]\nResponse from {device_name}:\n{start_ret.message}" @@ -254,10 +264,12 @@ def stop(args): ) with console.status("Stopping device...", spinner="bouncingBall"): - stop_ret = syn.Device(args.uri, args.verbose).stop_with_status() + stop_ret = syn.Device( + args.uri, args.verbose, raise_rpc_errors=True + ).stop_with_status() if not stop_ret: - console.print("[bold red]Internal error stopping device") - return + console.print("[bold red]Failed to stop device[/bold red]") + return False if stop_ret.code != StatusCode.kOk: console.print(f"[bold red]Error stopping\n{stop_ret.message}") return @@ -274,10 +286,12 @@ def configure(args): console.print("Configuring device with the following configuration:") console.print(config_obj.to_proto()) - config_ret = syn.Device(args.uri, args.verbose).configure_with_status(config_obj) + config_ret = syn.Device( + args.uri, args.verbose, raise_rpc_errors=True + ).configure_with_status(config_obj) if not config_ret: - console.print("[bold red]Internal error configuring device") - return + console.print("[bold red]Failed to configure device[/bold red]") + return False if config_ret.code != StatusCode.kOk: console.print(f"[bold red]Error configuring\n{config_ret.message}") return @@ -300,7 +314,7 @@ def parse_datetime(time_str: Optional[str]) -> Optional[datetime]: try: if args.follow: with console.status("Tailing logs...", spinner="bouncingBall"): - device = syn.Device(args.uri, args.verbose) + device = syn.Device(args.uri, args.verbose, raise_rpc_errors=True) for log in device.tail_logs(args.log_level): line = log_entry_to_str(log) if output_file: @@ -324,7 +338,9 @@ def parse_datetime(time_str: Optional[str]) -> Optional[datetime]: return with console.status("Getting logs...", spinner="bouncingBall"): - res = syn.Device(args.uri, args.verbose).get_logs_with_status( + res = syn.Device( + args.uri, args.verbose, raise_rpc_errors=True + ).get_logs_with_status( log_level=args.log_level, since_ms=args.since, start_time=start_time, @@ -349,7 +365,7 @@ def parse_datetime(time_str: Optional[str]) -> Optional[datetime]: def list_apps(args): console = Console() with console.status("Listing installed applications...", spinner="bouncingBall"): - device = syn.Device(args.uri, args.verbose) + device = syn.Device(args.uri, args.verbose, raise_rpc_errors=True) response = device.list_apps() if not response: diff --git a/synapse/cli/settings.py b/synapse/cli/settings.py index 109d2555..e740e4ae 100644 --- a/synapse/cli/settings.py +++ b/synapse/cli/settings.py @@ -3,6 +3,8 @@ from rich.console import Console from rich.table import Table +from synapse.cli.errors import print_error + def add_commands(subparsers): parser = subparsers.add_parser( @@ -29,7 +31,7 @@ def get_settings(args): try: with console.status("Getting settings", spinner="bouncingBall"): - device = syn.Device(args.uri, args.verbose) + device = syn.Device(args.uri, args.verbose, raise_rpc_errors=True) settings_dict = settings.get_all_settings(device) if not settings_dict: @@ -53,7 +55,8 @@ def get_settings(args): console.print(settings_table) except Exception as e: - console.print(f"[bold red]{e}[/bold red]") + print_error(console, e, context="Failed to get settings", verbose=args.verbose) + return False def set_setting(args): @@ -61,7 +64,7 @@ def set_setting(args): try: with console.status("Setting settings", spinner="bouncingBall"): - device = syn.Device(args.uri, args.verbose) + device = syn.Device(args.uri, args.verbose, raise_rpc_errors=True) updated_value = settings.set_setting(device, args.key, args.value) console.print( @@ -70,4 +73,7 @@ def set_setting(args): console.print(f"[dim]Confirmed value: {updated_value}[/dim]") except Exception as e: - console.print(f"[bold red]{e}[/bold red]") + print_error( + console, e, context="Failed to update setting", verbose=args.verbose + ) + return False diff --git a/synapse/cli/streaming.py b/synapse/cli/streaming.py index 0283822f..748472fc 100644 --- a/synapse/cli/streaming.py +++ b/synapse/cli/streaming.py @@ -15,6 +15,7 @@ from synapse.api.status_pb2 import DeviceState, StatusCode from synapse.api.node_pb2 import NodeType from synapse.client.taps import Tap +from synapse.cli.errors import print_error from synapse.utils.proto import load_device_config from synapse.utils.electrode_ids import ( GPIO_CHANNEL_ID_OFFSET, @@ -280,9 +281,7 @@ def set_attributes( # indices otherwise — `id_source` says which. The logical channel id and # channel type are written alongside it, as equal-length datasets, so # downstream analysis can reconstruct the full mapping. - electrodes_group.create_dataset( - "id", data=electrode_rows.ids, dtype="uint32" - ) + electrodes_group.create_dataset("id", data=electrode_rows.ids, dtype="uint32") electrodes_group.create_dataset( "channel_id", data=electrode_rows.channel_ids, dtype="uint32" ) @@ -615,7 +614,9 @@ def configure_device(device, config, console): # Apply the configuration to the device configure_status = device.configure_with_status(config) if configure_status is None: - console.print("[bold red]Failed to configure device: connection error[/bold red]") + console.print( + "[bold red]Failed to configure device: connection error[/bold red]" + ) return False if configure_status.code != StatusCode.kOk: console.print( @@ -635,7 +636,9 @@ def start_device(device, console): with console.status("Starting device...", spinner="bouncingBall"): start_status = device.start_with_status() if start_status is None: - console.print("[bold red]Failed to start device: connection error[/bold red]") + console.print( + "[bold red]Failed to start device: connection error[/bold red]" + ) return False if start_status.code != StatusCode.kOk: console.print( @@ -649,7 +652,9 @@ def stop_device(device, console): with console.status("Stopping device...", spinner="bouncingBall"): stop_status = device.stop_with_status() if stop_status is None: - console.print("[bold red]Failed to stop device: connection error[/bold red]") + console.print( + "[bold red]Failed to stop device: connection error[/bold red]" + ) return False if stop_status.code != StatusCode.kOk: console.print( @@ -734,9 +739,7 @@ def detect_stream_parameters(broadband_tap, console, channel_to_electrode=None): sample_rate = first_frame.sample_rate_hz electrode_rows = derive_electrode_row_ids(first_frame, channel_to_electrode) if electrode_rows is None: - console.print( - "[bold red]First message carried no channel data[/bold red]" - ) + console.print("[bold red]First message carried no channel data[/bold red]") return None, None, None num_channels = len(electrode_rows.ids) @@ -751,7 +754,7 @@ def detect_stream_parameters(broadband_tap, console, channel_to_electrode=None): return sample_rate, electrode_rows, first_frame except Exception as e: - console.print(f"[bold red]Error detecting stream parameters: {e}[/bold red]") + print_error(console, e, context="Failed to detect stream parameters") return None, None, None @@ -945,11 +948,11 @@ def read(args): try: config = load_device_config(args.config, console) except Exception as e: - console.print(f"[bold red]Failed to load device configuration: {e}[/bold red]") - return + print_error(console, e, context="Failed to load device configuration") + return False # Create the device object - device = syn.Device(args.uri, args.verbose) + device = syn.Device(args.uri, args.verbose, raise_rpc_errors=True) device_name = device.get_name() console.log(f"[green]Connected to {device_name}[/green]") @@ -1030,10 +1033,12 @@ def read(args): f"[green]Started real-time plotter with {len(available_channels)} channels available[/green]" ) except ImportError as e: - console.print( - f"[bold red]Failed to import plotter (missing dearpygui?): {e}[/bold red]" + print_error( + console, + e, + context="Failed to import plotter (is dearpygui installed?)", ) - return + return False # Setup stream monitor monitor = StreamMonitor(console) diff --git a/synapse/client/device.py b/synapse/client/device.py index 2350f7b2..66f7c6d4 100644 --- a/synapse/client/device.py +++ b/synapse/client/device.py @@ -29,7 +29,7 @@ class Device(object): - def __init__(self, uri, verbose=False): + def __init__(self, uri, verbose=False, raise_rpc_errors=False): if not uri: raise ValueError("URI cannot be empty or none") if len(uri.split(":")) != 2: @@ -39,6 +39,7 @@ def __init__(self, uri, verbose=False): self.channel = grpc.insecure_channel(self.uri) self.rpc = SynapseDeviceStub(self.channel) + self.raise_rpc_errors = raise_rpc_errors self.logger = logging.getLogger(__name__) level = logging.DEBUG if verbose else logging.ERROR @@ -50,7 +51,7 @@ def start(self): if self._handle_status_response(response): return response except grpc.RpcError as e: - self.logger.debug("Error: %s", e.details()) + self._handle_rpc_error(e) return False def start_with_status(self) -> Status: @@ -58,8 +59,8 @@ def start_with_status(self) -> Status: response = self.rpc.Start(Empty()) return response except grpc.RpcError as e: - self.logger.error("Error: %s", e.details()) - return None + self._handle_rpc_error(e) + return None def stop(self): try: @@ -67,14 +68,14 @@ def stop(self): if self._handle_status_response(response): return response except grpc.RpcError as e: - self.logger.error("Error: %s", e.details()) + self._handle_rpc_error(e) return False def stop_with_status(self) -> Status: try: return self.rpc.Stop(Empty()) except grpc.RpcError as e: - self.logger.error("Error: %s", e.details()) + self._handle_rpc_error(e) return None def info(self): @@ -83,7 +84,7 @@ def info(self): self._handle_status_response(response.status) return response except grpc.RpcError as e: - self.logger.error("Error: %s", e.details()) + self._handle_rpc_error(e) return None def query(self, query): @@ -91,7 +92,7 @@ def query(self, query): response = self.rpc.Query(query) return response except grpc.RpcError as e: - self.logger.error("Error: %s", e.details()) + self._handle_rpc_error(e) return None def configure(self, config: Config): @@ -103,7 +104,7 @@ def configure(self, config: Config): if self._handle_status_response(response): return response except grpc.RpcError as e: - self.logger.error("Error: %s", e.details()) + self._handle_rpc_error(e) return False def configure_with_status(self, config: Config) -> Status: @@ -114,7 +115,7 @@ def configure_with_status(self, config: Config) -> Status: response = self.rpc.Configure(config.to_proto()) return response except grpc.RpcError as e: - self.logger.error("Error: %s", e.details()) + self._handle_rpc_error(e) return None def get_name(self) -> Optional[str]: @@ -143,7 +144,7 @@ def get_logs( response = self.rpc.GetLogs(request) return response except grpc.RpcError as e: - self.logger.error("Error: %s", e.details()) + self._handle_rpc_error(e) return None def get_logs_with_status( @@ -167,7 +168,7 @@ def get_logs_with_status( return self.rpc.GetLogs(request) except grpc.RpcError as e: - self.logger.error("Error: %s", e.details()) + self._handle_rpc_error(e) return None def tail_logs( @@ -178,7 +179,7 @@ def tail_logs( request.min_level = log_level_to_pb(log_level) return self.rpc.TailLogs(request) except grpc.RpcError as e: - self.logger.error("Error: %s", e.details()) + self._handle_rpc_error(e) return None def stream_query( @@ -187,8 +188,11 @@ def stream_query( try: for response in self.rpc.StreamQuery(stream_request): yield response + except grpc.RpcError as e: + self._handle_rpc_error(e) + yield StreamQueryResponse(code=StatusCode.kQueryFailed) except Exception as e: - self.logger.error(f"Error during StreamQuery: {str(e)}") + self.logger.error("Stream query failed: %s", e) yield StreamQueryResponse(code=StatusCode.kQueryFailed) def update_device_settings( @@ -197,8 +201,11 @@ def update_device_settings( try: return self.rpc.UpdateDeviceSettings(request) + except grpc.RpcError as e: + self._handle_rpc_error(e) + return None except Exception as e: - self.logger.error(f"Error during update settings: {str(e)}") + self.logger.error("Settings update failed: %s", e) return None def list_apps(self) -> Optional[ListAppsResponse]: @@ -207,7 +214,7 @@ def list_apps(self) -> Optional[ListAppsResponse]: response = self.rpc.ListApps(ListAppsRequest()) return response except grpc.RpcError as e: - self.logger.error("Error listing apps: %s", e.details()) + self._handle_rpc_error(e) return None def _handle_status_response(self, status): @@ -216,3 +223,8 @@ def _handle_status_response(self, status): return False else: return True + + def _handle_rpc_error(self, error: grpc.RpcError) -> None: + if self.raise_rpc_errors: + raise error + self.logger.error("Error: %s", error.details()) diff --git a/synapse/tests/cli/test_errors.py b/synapse/tests/cli/test_errors.py new file mode 100644 index 00000000..2997a3bd --- /dev/null +++ b/synapse/tests/cli/test_errors.py @@ -0,0 +1,123 @@ +from io import StringIO +import logging + +import grpc +import pytest +from rich.console import Console + +from synapse.cli.errors import error_message, run_action +from synapse.client.config import Config +from synapse.client.device import Device + + +class FakeRpcError(grpc.RpcError): + def code(self): + return grpc.StatusCode.UNAUTHENTICATED + + def details(self): + return "not paired with this device; pair first" + + def __str__(self): + return ( + "<_MultiThreadedRendezvous status=UNAUTHENTICATED " + 'debug_error_string="internal transport details">' + ) + + +def capture_console(): + output = StringIO() + return Console(file=output, color_system=None), output + + +def test_rpc_error_message_hides_transport_diagnostics(): + message = error_message(FakeRpcError()) + + assert message == "not paired with this device; pair first" + assert "_MultiThreadedRendezvous" not in message + assert "debug_error_string" not in message + + +def test_verbose_rpc_error_message_includes_status_but_not_transport_repr(): + message = error_message(FakeRpcError(), verbose=True) + + assert message == ("gRPC UNAUTHENTICATED: not paired with this device; pair first") + assert "_MultiThreadedRendezvous" not in message + assert "debug_error_string" not in message + + +def test_semantic_error_does_not_print_usage(): + console, output = capture_console() + + def fail(): + raise ValueError("configuration is not valid for this device") + + exit_code = run_action(fail, console=console) + + assert exit_code == 1 + assert output.getvalue() == ("Error: configuration is not valid for this device\n") + assert "usage:" not in output.getvalue() + assert "Uncaught" not in output.getvalue() + + +def test_rpc_error_is_concise_and_fails(): + console, output = capture_console() + + def fail(): + raise FakeRpcError() + + exit_code = run_action(fail, console=console) + + assert exit_code == 1 + assert output.getvalue() == "Error: not paired with this device; pair first\n" + + +def test_false_command_result_is_a_failure(): + console, _ = capture_console() + + assert run_action(lambda: False, console=console) == 1 + + +def test_configure_rpc_error_reaches_cli_boundary_without_duplicate_message(): + class FailingRpc: + def Configure(self, _request): + raise FakeRpcError() + + device = Device.__new__(Device) + device.rpc = FailingRpc() + device.raise_rpc_errors = True + console, output = capture_console() + + exit_code = run_action( + lambda: device.configure_with_status(Config()), + console=console, + ) + + assert exit_code == 1 + assert output.getvalue() == "Error: not paired with this device; pair first\n" + assert "Internal error configuring device" not in output.getvalue() + + +def test_cli_device_configure_rpc_error_is_not_converted_to_none(): + class FailingRpc: + def Configure(self, _request): + raise FakeRpcError() + + device = Device.__new__(Device) + device.rpc = FailingRpc() + device.raise_rpc_errors = True + + with pytest.raises(FakeRpcError): + device.configure_with_status(Config()) + + +def test_default_device_configure_preserves_none_on_rpc_error(): + class FailingRpc: + def Configure(self, _request): + raise FakeRpcError() + + device = Device.__new__(Device) + device.rpc = FailingRpc() + device.raise_rpc_errors = False + device.logger = logging.getLogger(__name__) + + assert device.configure_with_status(Config()) is None From 53d448d9d1994a7914d3fe57fb1dbb06c78cff00 Mon Sep 17 00:00:00 2001 From: calvinleng-science Date: Thu, 17 Sep 2026 14:57:31 -0700 Subject: [PATCH 2/6] Add PR error verification script Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/prs/194-scary-errors.sh | 132 +++++++++++++++++++++++++++++++ tests/prs/194-scary-errors.sh.md | 16 ++++ 2 files changed, 148 insertions(+) create mode 100755 tests/prs/194-scary-errors.sh create mode 100644 tests/prs/194-scary-errors.sh.md diff --git a/tests/prs/194-scary-errors.sh b/tests/prs/194-scary-errors.sh new file mode 100755 index 00000000..3b32eea7 --- /dev/null +++ b/tests/prs/194-scary-errors.sh @@ -0,0 +1,132 @@ +#!/usr/bin/env bash +set -uo pipefail + +DEVICE="127.0.0.1" +SYNAPSECTL_BIN="${SYNAPSECTL:-}" + +while (($#)); do + case "$1" in + --device) + [[ $# -ge 2 ]] || { echo "--device requires an IP address" >&2; exit 2; } + DEVICE="$2" + shift 2 + ;; + --help) + echo "Usage: $0 [--device ]" + echo "Set SYNAPSECTL=/path/to/synapsectl to override executable discovery." + exit 0 + ;; + *) + echo "Unknown argument: $1" >&2 + exit 2 + ;; + esac +done + +if [[ -z "$SYNAPSECTL_BIN" ]]; then + SYNVENV_CTL="$HOME/Documents/venvs/synapse-python/bin/synapsectl" + if [[ -x "$SYNVENV_CTL" ]]; then + SYNAPSECTL_BIN="$SYNVENV_CTL" + else + SYNAPSECTL_BIN="$(command -v synapsectl || true)" + fi +fi + +if [[ -z "$SYNAPSECTL_BIN" || ! -x "$SYNAPSECTL_BIN" ]]; then + echo "synapsectl was not found; activate synvenv or set SYNAPSECTL." >&2 + exit 1 +fi + +TMP_DIR="$(mktemp -d)" +trap 'rm -rf "$TMP_DIR"' EXIT +touch "$TMP_DIR/not-json.txt" +mkdir "$TMP_DIR/empty-app" "$TMP_DIR/empty-peripheral" "$TMP_DIR/gateware-cwd" + +PASS=0 +FAIL=0 + +run_case() { + local label="$1" + local kind="$2" + shift 2 + local output status + + printf '\n=== %s ===\n' "$label" + printf 'Command:' + printf ' %q' "$SYNAPSECTL_BIN" "$@" + printf '\n' + + output="$(timeout 15 "$SYNAPSECTL_BIN" "$@" 2>&1)" + status=$? + printf '%s\n' "$output" + printf '[exit %d]\n' "$status" + + if grep -Eq 'Uncaught|_MultiThreadedRendezvous|debug_error_string|UNKNOWN:Error received from peer' <<<"$output"; then + echo "FAIL: raw/internal exception details were displayed" + FAIL=$((FAIL + 1)) + return + fi + + if [[ "$kind" == "semantic" ]] && grep -Eq '^usage:' <<<"$output"; then + echo "FAIL: semantic error printed usage" + FAIL=$((FAIL + 1)) + return + fi + + if [[ "$kind" == "usage" ]] && ! grep -Eq '^usage:' <<<"$output"; then + echo "FAIL: usage error did not print usage" + FAIL=$((FAIL + 1)) + return + fi + + if [[ -z "$output" ]]; then + echo "FAIL: command produced no message to inspect" + FAIL=$((FAIL + 1)) + return + fi + + echo "PASS" + PASS=$((PASS + 1)) +} + +rpc() { + run_case "$1" semantic --uri "$DEVICE" "${@:2}" +} + +run_case "discover: unknown option" usage discover --definitely-invalid +rpc "info: device RPC failure" info +run_case "query: missing query file" semantic query "$TMP_DIR/missing.json" +run_case "start: invalid configuration type" semantic start "$TMP_DIR/not-json.txt" +run_case "stop: extra argument" usage stop app-one app-two +run_case "configure: invalid configuration type" semantic configure "$TMP_DIR/not-json.txt" +run_case "logs: invalid timestamp" semantic logs --start-time not-a-timestamp +run_case "read: missing configuration" semantic read "$TMP_DIR/missing.json" +run_case "plot: missing HDF5 file" semantic plot --data "$TMP_DIR/missing.h5" + +run_case "file ls: unknown option" usage file ls --definitely-invalid +run_case "file get: missing remote path" usage file get +run_case "file rm: missing remote path" usage file rm +rpc "taps list: device RPC failure" taps list +rpc "taps stream: unavailable tap" taps stream __missing_tap__ + +run_case "apps build: missing manifest" semantic apps build "$TMP_DIR/empty-app" +run_case "apps deploy: missing package" semantic apps deploy --package "$TMP_DIR/missing.deb" +rpc "apps list: device RPC failure" apps list + +run_case "peripherals build driver: missing manifest" semantic peripherals build driver "$TMP_DIR/empty-peripheral" +run_case "peripherals build gateware: missing manifest" semantic peripherals build gateware "$TMP_DIR/empty-peripheral" +run_case "peripherals build both: missing manifest" semantic peripherals build both "$TMP_DIR/empty-peripheral" +run_case "peripherals deploy driver: missing package" semantic peripherals deploy driver --package "$TMP_DIR/missing.deb" +run_case "peripherals deploy gateware: missing package" semantic peripherals deploy gateware --package "$TMP_DIR/missing.deb" +run_case "peripherals deploy both: missing package" semantic peripherals deploy both --package "$TMP_DIR/missing.deb" +pushd "$TMP_DIR/gateware-cwd" >/dev/null +run_case "peripherals gateware: missing Dockerfile" semantic peripherals gateware doctor +popd >/dev/null + +rpc "settings get: device RPC failure" settings get +run_case "settings set: missing value" usage settings set __missing_key__ +run_case "deploy-model: missing model" semantic deploy-model "$TMP_DIR/missing.onnx" + +printf '\n=== Summary ===\n' +printf '%d passed, %d failed\n' "$PASS" "$FAIL" +((FAIL == 0)) diff --git a/tests/prs/194-scary-errors.sh.md b/tests/prs/194-scary-errors.sh.md new file mode 100644 index 00000000..4f7ea799 --- /dev/null +++ b/tests/prs/194-scary-errors.sh.md @@ -0,0 +1,16 @@ +# CLI error verification + +Run `tests/prs/194-scary-errors.sh --device `. The device-backed cases are +read-only; commands that could change device state fail during parsing or local +validation instead. + +- **L1-24** — parses `--device` and help arguments. +- **L26-43** — locates `synapsectl` and creates disposable invalid inputs. +- **L45-90** — runs each case, prints its complete output, and rejects raw + exception internals or usage text on semantic errors. +- **L92-94** — routes read-only RPC cases through the selected device. +- **L96-104** — checks every ungrouped core command. +- **L106-114** — checks all file, tap, and application subcommands. +- **L116-124** — checks every peripheral build, deploy, and gateware path. +- **L126-128** — checks settings and model deployment commands. +- **L130-132** — prints totals and fails if any message violated the rules. From 2f781dd2542632e607b2c11a95b2dfb384847681 Mon Sep 17 00:00:00 2001 From: calvinleng-science Date: Thu, 17 Sep 2026 15:24:15 -0700 Subject: [PATCH 3/6] Remove PR-specific verification artifacts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/prs/194-scary-errors.sh | 132 ------------------------------- tests/prs/194-scary-errors.sh.md | 16 ---- 2 files changed, 148 deletions(-) delete mode 100755 tests/prs/194-scary-errors.sh delete mode 100644 tests/prs/194-scary-errors.sh.md diff --git a/tests/prs/194-scary-errors.sh b/tests/prs/194-scary-errors.sh deleted file mode 100755 index 3b32eea7..00000000 --- a/tests/prs/194-scary-errors.sh +++ /dev/null @@ -1,132 +0,0 @@ -#!/usr/bin/env bash -set -uo pipefail - -DEVICE="127.0.0.1" -SYNAPSECTL_BIN="${SYNAPSECTL:-}" - -while (($#)); do - case "$1" in - --device) - [[ $# -ge 2 ]] || { echo "--device requires an IP address" >&2; exit 2; } - DEVICE="$2" - shift 2 - ;; - --help) - echo "Usage: $0 [--device ]" - echo "Set SYNAPSECTL=/path/to/synapsectl to override executable discovery." - exit 0 - ;; - *) - echo "Unknown argument: $1" >&2 - exit 2 - ;; - esac -done - -if [[ -z "$SYNAPSECTL_BIN" ]]; then - SYNVENV_CTL="$HOME/Documents/venvs/synapse-python/bin/synapsectl" - if [[ -x "$SYNVENV_CTL" ]]; then - SYNAPSECTL_BIN="$SYNVENV_CTL" - else - SYNAPSECTL_BIN="$(command -v synapsectl || true)" - fi -fi - -if [[ -z "$SYNAPSECTL_BIN" || ! -x "$SYNAPSECTL_BIN" ]]; then - echo "synapsectl was not found; activate synvenv or set SYNAPSECTL." >&2 - exit 1 -fi - -TMP_DIR="$(mktemp -d)" -trap 'rm -rf "$TMP_DIR"' EXIT -touch "$TMP_DIR/not-json.txt" -mkdir "$TMP_DIR/empty-app" "$TMP_DIR/empty-peripheral" "$TMP_DIR/gateware-cwd" - -PASS=0 -FAIL=0 - -run_case() { - local label="$1" - local kind="$2" - shift 2 - local output status - - printf '\n=== %s ===\n' "$label" - printf 'Command:' - printf ' %q' "$SYNAPSECTL_BIN" "$@" - printf '\n' - - output="$(timeout 15 "$SYNAPSECTL_BIN" "$@" 2>&1)" - status=$? - printf '%s\n' "$output" - printf '[exit %d]\n' "$status" - - if grep -Eq 'Uncaught|_MultiThreadedRendezvous|debug_error_string|UNKNOWN:Error received from peer' <<<"$output"; then - echo "FAIL: raw/internal exception details were displayed" - FAIL=$((FAIL + 1)) - return - fi - - if [[ "$kind" == "semantic" ]] && grep -Eq '^usage:' <<<"$output"; then - echo "FAIL: semantic error printed usage" - FAIL=$((FAIL + 1)) - return - fi - - if [[ "$kind" == "usage" ]] && ! grep -Eq '^usage:' <<<"$output"; then - echo "FAIL: usage error did not print usage" - FAIL=$((FAIL + 1)) - return - fi - - if [[ -z "$output" ]]; then - echo "FAIL: command produced no message to inspect" - FAIL=$((FAIL + 1)) - return - fi - - echo "PASS" - PASS=$((PASS + 1)) -} - -rpc() { - run_case "$1" semantic --uri "$DEVICE" "${@:2}" -} - -run_case "discover: unknown option" usage discover --definitely-invalid -rpc "info: device RPC failure" info -run_case "query: missing query file" semantic query "$TMP_DIR/missing.json" -run_case "start: invalid configuration type" semantic start "$TMP_DIR/not-json.txt" -run_case "stop: extra argument" usage stop app-one app-two -run_case "configure: invalid configuration type" semantic configure "$TMP_DIR/not-json.txt" -run_case "logs: invalid timestamp" semantic logs --start-time not-a-timestamp -run_case "read: missing configuration" semantic read "$TMP_DIR/missing.json" -run_case "plot: missing HDF5 file" semantic plot --data "$TMP_DIR/missing.h5" - -run_case "file ls: unknown option" usage file ls --definitely-invalid -run_case "file get: missing remote path" usage file get -run_case "file rm: missing remote path" usage file rm -rpc "taps list: device RPC failure" taps list -rpc "taps stream: unavailable tap" taps stream __missing_tap__ - -run_case "apps build: missing manifest" semantic apps build "$TMP_DIR/empty-app" -run_case "apps deploy: missing package" semantic apps deploy --package "$TMP_DIR/missing.deb" -rpc "apps list: device RPC failure" apps list - -run_case "peripherals build driver: missing manifest" semantic peripherals build driver "$TMP_DIR/empty-peripheral" -run_case "peripherals build gateware: missing manifest" semantic peripherals build gateware "$TMP_DIR/empty-peripheral" -run_case "peripherals build both: missing manifest" semantic peripherals build both "$TMP_DIR/empty-peripheral" -run_case "peripherals deploy driver: missing package" semantic peripherals deploy driver --package "$TMP_DIR/missing.deb" -run_case "peripherals deploy gateware: missing package" semantic peripherals deploy gateware --package "$TMP_DIR/missing.deb" -run_case "peripherals deploy both: missing package" semantic peripherals deploy both --package "$TMP_DIR/missing.deb" -pushd "$TMP_DIR/gateware-cwd" >/dev/null -run_case "peripherals gateware: missing Dockerfile" semantic peripherals gateware doctor -popd >/dev/null - -rpc "settings get: device RPC failure" settings get -run_case "settings set: missing value" usage settings set __missing_key__ -run_case "deploy-model: missing model" semantic deploy-model "$TMP_DIR/missing.onnx" - -printf '\n=== Summary ===\n' -printf '%d passed, %d failed\n' "$PASS" "$FAIL" -((FAIL == 0)) diff --git a/tests/prs/194-scary-errors.sh.md b/tests/prs/194-scary-errors.sh.md deleted file mode 100644 index 4f7ea799..00000000 --- a/tests/prs/194-scary-errors.sh.md +++ /dev/null @@ -1,16 +0,0 @@ -# CLI error verification - -Run `tests/prs/194-scary-errors.sh --device `. The device-backed cases are -read-only; commands that could change device state fail during parsing or local -validation instead. - -- **L1-24** — parses `--device` and help arguments. -- **L26-43** — locates `synapsectl` and creates disposable invalid inputs. -- **L45-90** — runs each case, prints its complete output, and rejects raw - exception internals or usage text on semantic errors. -- **L92-94** — routes read-only RPC cases through the selected device. -- **L96-104** — checks every ungrouped core command. -- **L106-114** — checks all file, tap, and application subcommands. -- **L116-124** — checks every peripheral build, deploy, and gateware path. -- **L126-128** — checks settings and model deployment commands. -- **L130-132** — prints totals and fails if any message violated the rules. From 9a91f197586b2fcbd83fe0b6401eea800bba4b6c Mon Sep 17 00:00:00 2001 From: calvinleng-science Date: Thu, 17 Sep 2026 15:50:28 -0700 Subject: [PATCH 4/6] Avoid duplicate SSH authentication errors Let the CLI render expected SSH failures without first emitting a root logger diagnostic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- synapse/client/sftp.py | 19 +++++++++++-------- synapse/tests/client/test_sftp.py | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 8 deletions(-) create mode 100644 synapse/tests/client/test_sftp.py diff --git a/synapse/client/sftp.py b/synapse/client/sftp.py index c4d55253..e7bff642 100644 --- a/synapse/client/sftp.py +++ b/synapse/client/sftp.py @@ -3,17 +3,20 @@ import paramiko import paramiko.ssh_exception -def connect_sftp(hostname, username, password=None, pass_filename=None, key_filename=None, port=22): + +def connect_sftp( + hostname, username, password=None, pass_filename=None, key_filename=None, port=22 +): """ Connect to SFTP server and return SFTP client object - + Args: hostname: SFTP server hostname or IP username: Username for authentication password: Password for authentication (optional if using key) key_filename: Path to private key file (optional if using password) port: SFTP server port (default: 22) - + Returns: tuple: (SSHClient, SFTPClient) - Keep both to properly close connection """ @@ -40,22 +43,22 @@ def connect_sftp(hostname, username, password=None, pass_filename=None, key_file look_for_keys=False, ) sftp = ssh.open_sftp() - except TimeoutError as e: + except TimeoutError: logging.error(f"Connection to {hostname} timed out") return None, None except socket.error as e: logging.error(f"Socket error connecting to {hostname}:{port}: {e}") return None, None - except paramiko.ssh_exception.SSHException as e: - logging.error(f"SSH error connecting to {hostname}:{port}: {e}") - raise # Re-raise to let caller handle it + except paramiko.ssh_exception.SSHException: + raise return ssh, sftp + def close_sftp(ssh, sftp): """ Close SFTP connection - + Args: ssh: SSHClient object sftp: SFTPClient object diff --git a/synapse/tests/client/test_sftp.py b/synapse/tests/client/test_sftp.py new file mode 100644 index 00000000..b551d6c5 --- /dev/null +++ b/synapse/tests/client/test_sftp.py @@ -0,0 +1,19 @@ +import logging + +import paramiko +import pytest + +from synapse.client import sftp + + +def test_authentication_failure_is_not_logged_before_reraising(monkeypatch, caplog): + def fail_authentication(self, **kwargs): + raise paramiko.AuthenticationException("Authentication failed.") + + monkeypatch.setattr(paramiko.SSHClient, "connect", fail_authentication) + + with caplog.at_level(logging.ERROR): + with pytest.raises(paramiko.AuthenticationException): + sftp.connect_sftp("192.0.2.1", "user", "bad-password") + + assert caplog.records == [] From 9cec209a1367575517d6bc4b3819c139281b7cfd Mon Sep 17 00:00:00 2001 From: calvinleng-science Date: Thu, 17 Sep 2026 16:01:26 -0700 Subject: [PATCH 5/6] Validate plot inputs before launching Qt Report missing data and legacy configuration files directly instead of surfacing NoneType errors or unrelated display warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- synapse/cli/offline_plot.py | 45 ++++++++++++++++--- synapse/tests/cli/test_offline_plot_errors.py | 45 +++++++++++++++++++ 2 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 synapse/tests/cli/test_offline_plot_errors.py diff --git a/synapse/cli/offline_plot.py b/synapse/cli/offline_plot.py index 8670e87f..46f2a213 100644 --- a/synapse/cli/offline_plot.py +++ b/synapse/cli/offline_plot.py @@ -160,15 +160,32 @@ def compute_fft(data, sample_rate): def plot(args): logger = setup_logging() + console = Console() # NOTE(gilbert): we want to support the previous plotting code but we are moving to hdf5 saving and plotting # Short circuit for now and just use the hdf5 plotting code if args.data is not None: + if not os.path.isfile(args.data): + console.print( + f"[bold red]Error:[/bold red] Data file not found: {args.data}" + ) + return False _, file_extension = os.path.splitext(args.data) if file_extension == ".h5": return plot_h5(args) + if file_extension not in (".bin", ".dat", ".jsonl"): + console.print( + "[bold red]Error:[/bold red] Unsupported data file format. " + "Expected .h5, .bin, .dat, or .jsonl." + ) + return False + + if args.dir and not os.path.isdir(args.dir): + console.print( + f"[bold red]Error:[/bold red] Recording directory not found: {args.dir}" + ) + return False - console = Console() console.print( "[yellow bold]Legacy plotting is deprecated, please use the hdf5 files going forward[/yellow bold]" ) @@ -176,10 +193,6 @@ def plot(args): "[yellow bold]Use --data to plot hdf5 files[/yellow bold]" ) - app = QtWidgets.QApplication.instance() - if not app: - app = QtWidgets.QApplication(sys.argv) - data_file = None config_file = None if args.dir: @@ -199,6 +212,28 @@ def plot(args): if args.config: config_file = args.config + if data_file is None: + console.print( + "[bold red]Error:[/bold red] No recording data found. " + "Specify --data or --dir." + ) + return False + if config_file is None: + console.print( + "[bold red]Error:[/bold red] Legacy recordings require a " + "configuration file. Specify --config or use --dir." + ) + return False + if not os.path.isfile(config_file): + console.print( + f"[bold red]Error:[/bold red] Configuration file not found: {config_file}" + ) + return False + + app = QtWidgets.QApplication.instance() + if not app: + app = QtWidgets.QApplication(sys.argv) + # Start with loading the config sampling_freq, num_channels, channel_ids = load_config(config_file) if args.channels: diff --git a/synapse/tests/cli/test_offline_plot_errors.py b/synapse/tests/cli/test_offline_plot_errors.py new file mode 100644 index 00000000..86e7858e --- /dev/null +++ b/synapse/tests/cli/test_offline_plot_errors.py @@ -0,0 +1,45 @@ +from types import SimpleNamespace + +from synapse.cli import offline_plot + + +def plot_args(**overrides): + values = { + "data": None, + "config": None, + "time": None, + "channels": None, + "dir": None, + } + values.update(overrides) + return SimpleNamespace(**values) + + +def test_missing_data_file_is_reported_before_initializing_qt(monkeypatch, capsys): + monkeypatch.setattr( + offline_plot.QtWidgets.QApplication, + "instance", + lambda: (_ for _ in ()).throw(AssertionError("Qt should not initialize")), + ) + + result = offline_plot.plot(plot_args(data="asdfsadf")) + + assert result is False + assert "Error: Data file not found: asdfsadf" in capsys.readouterr().out + + +def test_legacy_data_requires_configuration(monkeypatch, tmp_path, capsys): + data_file = tmp_path / "recording.dat" + data_file.touch() + monkeypatch.setattr( + offline_plot.QtWidgets.QApplication, + "instance", + lambda: (_ for _ in ()).throw(AssertionError("Qt should not initialize")), + ) + + result = offline_plot.plot(plot_args(data=str(data_file))) + + assert result is False + output = capsys.readouterr().out + assert "Legacy recordings require a configuration file" in output + assert "NoneType" not in output From b30651cf9b760932a5d3ac525c50b7c08b85bbf3 Mon Sep 17 00:00:00 2001 From: calvinleng-science Date: Thu, 17 Sep 2026 16:04:41 -0700 Subject: [PATCH 6/6] Bump version to 2.7.8 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 90a09732..e8db118c 100644 --- a/setup.py +++ b/setup.py @@ -29,7 +29,7 @@ def stamp_api_version(): setup( name="science-synapse", - version="2.7.7", + version="2.7.8", description="Client library and CLI for the Synapse API", author="Science Team", author_email="team@science.xyz",