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

@@ -0,0 +1,3 @@
+from core.utils.run_commands import test_broken_pipe_on_process_death
+
+test_broken_pipe_on_process_death()

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 (
+from core.services.prepare_tickets.ticket_tracker import does_ticket_tracker_exist
-    get_data_for_a_single_ticket,
+
-    does_ticket_tracker_exist,
+# from core.services.prepare_tickets import setup_ticket_tracker
-)
-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,22 +46,26 @@ 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}
-    else:
+
-        try:
     if on_or_off == "on":
-                update_json(filepath, "use_random", True)
+        update_result = update_json(filepath, "use_random", True)
-                return {"valid": True}
+        return conclude_and_return(update_result=update_result, filepath=filepath, ticket_observer=ticket_observer)
     elif on_or_off == "off":
-                update_json(filepath, "use_random", False)
+        update_result = update_json(filepath, "use_random", False)
-                return {"valid": True}
+        return conclude_and_return(update_result=update_result, filepath=filepath, ticket_observer=ticket_observer)
     else:
         ticket_observer.notify("failed_input", None)
         return {
             "valid": False,
-                    "message": f"Invalid choice for turning on or off",
+            "message": "Invalid choice for turning on or off",
         }
-        except:
+
-            notification = f"Error with modifying config file.  Check {filepath}"
+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,18 +83,26 @@ 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:
+    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"]
 
-        # they want a random ticket
+    # if it read the config, but the value is false:
-        if random_setting == True:
+    if not random_setting:
-            data_results = pick_a_random_ticket(ticket_observer)
+        which_ticket = None
-
-            if "random_ticket" in data_results:
-                which_ticket = data_results["random_ticket"]
         error_msg = None
         return which_ticket, error_msg
-            else:
+
+    # 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"]
@@ -102,19 +112,11 @@ def do_we_use_a_random_ticket(ticket_observer: TicketObserver) -> tuple:
             )
         return which_ticket, error_msg
 
-        # if it read the config, but the value is false:
+    # finally get the result:
-        else:
+    which_ticket = data_results["random_ticket"]
-            which_ticket = None
     error_msg = None
     return which_ticket, error_msg
 
-    # this is a problem with reading the config itself:
-    else:
-        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
-
 
 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,7 +175,7 @@ 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)
@@ -211,14 +191,14 @@ def use_ticket(
 def pick_a_random_ticket(ticket_observer: TicketObserver) -> dict:
     ticket_data = get_unused_tickets(ticket_observer)
 
-    if "valid" in ticket_data:
+    if "valid" not 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:
         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}

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."""
-    Execute a command via persistent bash session.
-    Returns (returncode, stdout, stderr)
-    """
     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()
 
+    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
+        # Use select to wait for data with timeout
-            if not line:
+        ready, _, _ = select.select([terminal.stdout], [], [], 0.1)
-                time.sleep(0.02)
-                continue
         
+        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')
             
-            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
 
-        # 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
-