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", 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/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/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/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/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 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 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 == []