From 0983d1a68dff3e606ea39ffb973e2275e3cea793 Mon Sep 17 00:00:00 2001 From: SimplifiedPrivacy Date: Mon, 24 Aug 2026 11:24:23 -0400 Subject: [PATCH] Changed bash generic commands to no longer use blocking methods to search through the terminal output to help with broken pipe errors, and instead select. But kept it non-blocking for the entire terminal itself. Also refactored Use Ticket Controller to reduce nesting if statements. --- broken_pipe_test.py | 3 + .../tickets/UseTicketController.py | 172 ++++++++---------- core/utils/run_commands.py | 87 +++++---- 3 files changed, 130 insertions(+), 132 deletions(-) create mode 100644 broken_pipe_test.py diff --git a/broken_pipe_test.py b/broken_pipe_test.py new file mode 100644 index 0000000..d4608b2 --- /dev/null +++ b/broken_pipe_test.py @@ -0,0 +1,3 @@ +from core.utils.run_commands import test_broken_pipe_on_process_death + +test_broken_pipe_on_process_death() diff --git a/core/controllers/tickets/UseTicketController.py b/core/controllers/tickets/UseTicketController.py index 0e2134d..57743c3 100644 --- a/core/controllers/tickets/UseTicketController.py +++ b/core/controllers/tickets/UseTicketController.py @@ -6,15 +6,13 @@ if TYPE_CHECKING: from core.essentials.observers.ConnectionObserver import ConnectionObserver from core.Constants import Constants -from core.observers.BaseObserver import BaseObserver +# from core.observers.BaseObserver import BaseObserver from core.services.using_tickets.use_ticket_orchestrator import use_ticket_orchestrator -from core.services.networking.api_requests.ApiResponseModel import ApiResponse, ErrorType +from core.services.networking.api_requests.ApiResponseModel import ApiResponse -from core.services.prepare_tickets.ticket_tracker import ( - get_data_for_a_single_ticket, - does_ticket_tracker_exist, -) -from core.services.prepare_tickets import setup_ticket_tracker +from core.services.prepare_tickets.ticket_tracker import does_ticket_tracker_exist + +# from core.services.prepare_tickets import setup_ticket_tracker from core.services.helpers.does_ticket_file_exist import does_ticket_file_exist from core.services.helpers.get_value_from_config import get_value_from_config @@ -38,7 +36,7 @@ def modify_random_tickets_setting( choices = ["on", "off"] if on_or_off not in choices: ticket_observer.notify("failed_input", None) - return {"valid": False, "message": f"Invalid choice for turning on or off"} + return {"valid": False, "message": "Invalid choice for turning on or off"} billing_folder = Constants.HV_TICKETING_CONFIG_HOME filepath = f"{billing_folder}/billing_choices.json" @@ -48,24 +46,28 @@ def modify_random_tickets_setting( notification = "First setup the Tickets before picking use" ticket_observer.notify("failed_input", subject=notification) return {"valid": False, "message": notification} + + if on_or_off == "on": + update_result = update_json(filepath, "use_random", True) + return conclude_and_return(update_result=update_result, filepath=filepath, ticket_observer=ticket_observer) + elif on_or_off == "off": + update_result = update_json(filepath, "use_random", False) + return conclude_and_return(update_result=update_result, filepath=filepath, ticket_observer=ticket_observer) else: - try: - if on_or_off == "on": - update_json(filepath, "use_random", True) - return {"valid": True} - elif on_or_off == "off": - update_json(filepath, "use_random", False) - return {"valid": True} - else: - ticket_observer.notify("failed_input", None) - return { - "valid": False, - "message": f"Invalid choice for turning on or off", - } - except: - notification = f"Error with modifying config file. Check {filepath}" - ticket_observer.notify("error", subject=notification) - return {"valid": False, "message": notification} + ticket_observer.notify("failed_input", None) + return { + "valid": False, + "message": "Invalid choice for turning on or off", + } + +def conclude_and_return(update_result: bool, filepath: str, ticket_observer: TicketObserver) -> dict: + if update_result: + return {"valid": True} + else: + notification = f"Error with modifying config file. Check the filepath {filepath}" + logger.error(notification) + ticket_observer.notify("error", subject=notification) + return {"valid": False, "message": notification} """ @@ -81,40 +83,40 @@ def do_we_use_a_random_ticket(ticket_observer: TicketObserver) -> tuple: config_data = get_value_from_config("use_random") # if the 'value' key is in the config, that means it successfully read the config. - if "value" in config_data: - random_setting = config_data["value"] - - # they want a random ticket - if random_setting == True: - data_results = pick_a_random_ticket(ticket_observer) - - if "random_ticket" in data_results: - which_ticket = data_results["random_ticket"] - error_msg = None - return which_ticket, error_msg - else: - which_ticket = "error" - if "message" in data_results: - error_msg = data_results["message"] - else: - error_msg = ( - "Missing or Invalid Data. Unable to get unused ticket list." - ) - return which_ticket, error_msg - - # if it read the config, but the value is false: - else: - which_ticket = None - error_msg = None - return which_ticket, error_msg - - # this is a problem with reading the config itself: - else: + if "value" not in config_data: + # this is a problem with reading the config itself: which_ticket = "error" error_msg = "There is an error with the config file, or no config. Are you sure you have tickets?" ticket_observer.notify("failed_input", subject=error_msg) return which_ticket, error_msg + random_setting = config_data["value"] + + # if it read the config, but the value is false: + if not random_setting: + which_ticket = None + error_msg = None + return which_ticket, error_msg + + # they want a random ticket + data_results = pick_a_random_ticket(ticket_observer) + + # invalid format: + if "random_ticket" not in data_results: + which_ticket = "error" + if "message" in data_results: + error_msg = data_results["message"] + else: + error_msg = ( + "Missing or Invalid Data. Unable to get unused ticket list." + ) + return which_ticket, error_msg + + # finally get the result: + which_ticket = data_results["random_ticket"] + error_msg = None + return which_ticket, error_msg + def get_unused_tickets(ticket_observer: TicketObserver) -> dict: # does the file keeping track of ALL tickets exist: @@ -148,19 +150,12 @@ def use_ticket( profile: Optional[Union[SessionProfile, SystemProfile]] = None, ) -> dict: - print(f"Calling the use_ticket function! with which_ticket as {which_ticket}") - which_ticket = str(which_ticket) # type: ignore - print(f"were in use_ticket with ticket {which_ticket}") - if profile: - print(f"we have a profile of {profile}") - - # does the ticket's file exist: ticket_exists = does_ticket_file_exist(which_ticket) if ticket_exists == False: - error_msg = f"The ticket file does not exist in the correct folder." + error_msg = "The ticket file does not exist in the correct folder." ticket_observer.notify("failed_input", subject=error_msg) return {"valid": False, "message": error_msg} @@ -171,21 +166,6 @@ def use_ticket( ticket_observer.notify("failed_input", subject=error_msg) return {"valid": False, "message": error_msg} - # is the ticket used? - - # first, get values, - # try: - # status, location, subscription = get_data_for_a_single_ticket(which_ticket) - # if status == "used": - # error_msg = f"Ticket is already tied to {location} with the subscription {subscription}" - # ticket_observer.notify("failed_input", subject=error_msg) - # return {"valid": False, "message": error_msg} - # except ValueError as e: - # error_msg = f"Your local ticket tracker has no value for ticket {which_ticket}" - # logger.error(error_msg) - # logger.error(str(e)) - # return {"valid": False, "message": error_msg} - # the actual work here, everything else is just handling: ticket_observer.notify("connecting", "Connecting..") reply = use_ticket_orchestrator(which_ticket, which_location, connection_observer) @@ -195,30 +175,30 @@ def use_ticket( error_msg = reply.message reply_as_dict = {"valid": False, "message": f"API Failed: {error_msg}"} return reply_as_dict - else: - # this is dict in theory, - if isinstance(reply, dict): - valid = reply.get("valid", False) - # then if it worked, update the profile to save the ticket, - if valid and profile: - profile.ticket = which_ticket - profile.save() - logger.info(f"Saved ticket {which_ticket} to profile {profile.id}!") - return reply + # this is dict in theory, + if isinstance(reply, dict): + valid = reply.get("valid", False) + # then if it worked, update the profile to save the ticket, + if valid and profile: + profile.ticket = which_ticket + profile.save() + logger.info(f"Saved ticket {which_ticket} to profile {profile.id}!") + + return reply def pick_a_random_ticket(ticket_observer: TicketObserver) -> dict: ticket_data = get_unused_tickets(ticket_observer) - if "valid" in ticket_data: - if ticket_data["valid"] == True: - list_of_unused_tickets = ticket_data["data"] - - random_ticket = random.choice(list_of_unused_tickets) - return {"valid": True, "random_ticket": random_ticket} - else: - return ticket_data - else: + if "valid" not in ticket_data: error_msg = "Missing or Invalid Data. Unable to get unused ticket list." return {"valid": False, "message": error_msg} + + if ticket_data["valid"] != True: + return ticket_data + + # it's valid, get the list & pick one: + list_of_unused_tickets = ticket_data["data"] + random_ticket = random.choice(list_of_unused_tickets) + return {"valid": True, "random_ticket": random_ticket} diff --git a/core/utils/run_commands.py b/core/utils/run_commands.py index 7a2dec9..e534966 100644 --- a/core/utils/run_commands.py +++ b/core/utils/run_commands.py @@ -62,58 +62,73 @@ def _run_command_via_terminal( command: list, timeout: float | None = None ) -> tuple[int, str]: - """ - Execute a command via persistent bash session. - Returns (returncode, stdout, stderr) - """ + """Execute a command via persistent bash session.""" try: terminal = get_terminal() except RuntimeError: init_terminal() terminal = get_terminal() - # Convert command list to properly quoted bash string cmd_str = ' '.join(shlex.quote(arg) for arg in command) - - # Use a marker to detect command completion - # Redirect stderr to stdout and append marker with exit code marker = "::__CMD_DONE__::" - # wrapped_cmd = f"{cmd_str} 2>&1; echo \"{marker}$?\"\n" wrapped_cmd = f"{cmd_str} 2>&1; echo {marker}$?\n" + + # Check that process is still alive + if terminal.poll() is not None: + logger.error("Bash session died unexpectedly") + close_terminal() + init_terminal() + terminal = get_terminal() - - terminal.stdin.write(wrapped_cmd) - terminal.stdin.flush() + try: + terminal.stdin.write(wrapped_cmd) + terminal.stdin.flush() + except BrokenPipeError: + logger.error("Broken pipe on write—bash session is dead") + close_terminal() + raise RuntimeError("Bash session died") output_lines = [] returncode = 1 - timeout = timeout or float('inf') start_time = time.time() + buffer = "" # Accumulate incomplete lines - try: - while time.time() - start_time < timeout: - line = terminal.stdout.readline() # non-blocking due to init fcntl - if not line: - time.sleep(0.02) - continue + while time.time() - start_time < timeout: + # Use select to wait for data with timeout + ready, _, _ = select.select([terminal.stdout], [], [], 0.1) + + if ready: + try: + chunk = terminal.stdout.read(4096) # Read in chunks + if not chunk: # EOF + logger.error("Bash stdout closed unexpectedly") + close_terminal() + raise RuntimeError("Bash stdout closed") + + buffer += chunk + lines = buffer.split('\n') + buffer = lines[-1] # Keep incomplete line in buffer + + for line in lines[:-1]: + if marker in line: + returncode, before = extract_returncode(line, marker) + if before: + output_lines.append(before) + return returncode, ''.join(output_lines) + output_lines.append(line + '\n') - if marker in line: - returncode, before = extract_returncode(line, marker) - if before: - output_lines.append(before) - return returncode, ''.join(output_lines) - - output_lines.append(line) + except Exception as e: + logger.error(f"Error reading from stdout: {e}") + close_terminal() + raise + + # If no data ready, loop continues and we check timeout + + logger.error(f"Command timed out after {timeout}s") + close_terminal() + raise TimeoutError(f"Command exceeded {timeout}s limit") - # If we exit the loop without returning first, timeout occurred - logger.error(f"Command timed out after {timeout}s") - close_terminal() - raise TimeoutError(f"Command exceeded {timeout}s limit") - except Exception as e: - logger.error(f"We hit an error in the loop of reading, so we'll close the terminal, then the caller can deal with {e}") - close_terminal() - raise def run_generic_command( @@ -131,13 +146,14 @@ def run_generic_command( try: returncode, stdout = _run_command_via_terminal(command, timeout) + print("got output") + # output_data = stdout if returncode == 0: logger.info(f"{human_readable_goal} was successful") return Result(valid=True, data=stdout) else: - # Try stderr first, fallback to stdout for error parsing error_output = stdout.strip() error_enum = parse_errors(error_output) logger.error(f"{human_readable_goal} Failed, error type is {error_enum}.") @@ -172,4 +188,3 @@ def parse_errors(error_output: str) -> ResultError: else: return ResultError.UNKNOWN -