Fix crash, hang, and resource-leak defects found in a project-wide scan

je_web_runner/__init__.py

@@ -24,6 +24,9 @@
 from je_web_runner.utils.generate_report.generate_junit_xml_report import generate_junit_xml_report
 from je_web_runner.utils.generate_report.generate_allure_report import generate_allure
 from je_web_runner.utils.generate_report.generate_allure_report import generate_allure_report
+from je_web_runner.utils.socket_server.web_runner_socket_server import encode_frame
+from je_web_runner.utils.socket_server.web_runner_socket_server import read_frame
+from je_web_runner.utils.socket_server.web_runner_socket_server import send_command
 from je_web_runner.utils.socket_server.web_runner_socket_server import start_web_runner_socket_server
 from je_web_runner.utils.test_object.test_object_class import TestObject
 from je_web_runner.utils.test_object.test_object_class import create_test_object
@@ -387,6 +390,7 @@
     "generate_junit_xml", "generate_junit_xml_report",
     "generate_allure", "generate_allure_report",
     "start_web_runner_socket_server", "get_dir_files_as_list",
+    "send_command", "read_frame", "encode_frame",
     "TestObject", "create_test_object", "get_test_object_type_list",
     "test_record_instance", "Keys", "callback_executor", "create_project_dir",
     "load_env", "get_env", "expand_in_action", "EnvConfigError",

je_web_runner/manager/webrunner_manager.py

@@ -76,15 +76,36 @@ def change_webdriver(self, index_of_webdriver: int) -> None:
                 f"WebdriverManager change_webdriver, index_of_webdriver: {index_of_webdriver}, failed: {error!r}")
             record_action_to_list("web runner manager change_webdriver", param, error)
 
+    def _detach_wrapper_if_active(self, driver: WebDriver) -> None:
+        """
+        若共用的 wrapper 單例正指向這個已關閉的 driver，就把它解除綁定。
+        Clear the shared ``webdriver_wrapper`` singleton when it is still
+        pointing at ``driver``. The manager owns raw Selenium handles, so
+        closing one here would otherwise leave the wrapper (and its
+        ActionChains) bound to a dead session for the next caller.
+        """
+        if self.webdriver_wrapper.current_webdriver is driver:
+            self.webdriver_wrapper.set_active_driver(None)
+
     def close_current_webdriver(self) -> None:
         """
         關閉當前 WebDriver
         Close the current WebDriver
         """
         web_runner_logger.info("WebdriverManager close_current_webdriver")
         try:
-            self._current_webdriver_list.remove(self.current_webdriver)
-            self.current_webdriver.close()
+            if self.current_webdriver is None:
+                raise WebRunnerWebDriverIsNoneException(selenium_wrapper_web_driver_not_found_error)
+            # Close first, then drop the reference: if close() raises, the
+            # driver stays tracked so a later quit() can still reclaim it.
+            closed = self.current_webdriver
+            closed.close()
+            if closed in self._current_webdriver_list:
+                self._current_webdriver_list.remove(closed)
+            # Never leave ``current_webdriver`` pointing at a closed session —
+            # the next action would fail against a dead driver.
+            self.current_webdriver = None
+            self._detach_wrapper_if_active(closed)
             record_action_to_list("web runner manager close_current_webdriver", None, None)
         except Exception as error:
             web_runner_logger.error(f"WebdriverManager close_current_webdriver, failed: {error!r}")
@@ -100,9 +121,12 @@ def close_choose_webdriver(self, webdriver_index: int) -> None:
         web_runner_logger.info("WebdriverManager close_choose_webdriver")
         param = locals()
         try:
-            self.current_webdriver = self._current_webdriver_list[webdriver_index]
-            self.current_webdriver.close()
-            self._current_webdriver_list.remove(self._current_webdriver_list[webdriver_index])
+            chosen = self._current_webdriver_list[webdriver_index]
+            chosen.close()
+            self._current_webdriver_list.pop(webdriver_index)
+            if self.current_webdriver is chosen:
+                self.current_webdriver = None
+            self._detach_wrapper_if_active(chosen)
             record_action_to_list("web runner manager close_choose_webdriver", param, None)
         except Exception as error:
             web_runner_logger.error(f"WebdriverManager close_choose_webdriver, failed: {error!r}")
@@ -122,12 +146,28 @@ def quit(self) -> None:
             # Clean test records
             test_object_record.clean_record()
 
-            # 關閉所有 WebDriver
-            # Quit all WebDrivers
+            # 關閉所有 WebDriver；單一 driver 失敗不可中斷其餘的清理，
+            # 否則剩下的瀏覽器程序會變成 orphan。
+            # Quit every WebDriver. A failure on one must not abort the loop,
+            # or the remaining browsers leak as orphan processes.
+            quit_errors: list[Exception] = []
             for webdriver in self._current_webdriver_list:
-                webdriver.quit()
+                try:
+                    webdriver.quit()
+                except Exception as error:  # pylint: disable=broad-except
+                    web_runner_logger.error(f"WebdriverManager quit driver failed: {error!r}")
+                    quit_errors.append(error)
 
             self._current_webdriver_list = []
+            self.current_webdriver = None
+            # Every driver is gone, so the shared wrapper singleton must not
+            # keep serving a dead one to the next caller.
+            self.webdriver_wrapper.set_active_driver(None)
+            if quit_errors:
+                raise WebDriverException(
+                    f"WebdriverManager quit failed for {len(quit_errors)} driver(s): "
+                    f"{[repr(e) for e in quit_errors]}"
+                )
             record_action_to_list("web runner manager quit", None, None)
         except Exception as error:
             web_runner_logger.error(f"WebdriverManager quit, failed: {error!r}")

je_web_runner/utils/browser_pool/pool.py

@@ -89,12 +89,31 @@ def checkout(self, timeout: float = 30.0) -> PooledSession:
         if self._closed:
             raise BrowserPoolError("pool is closed")
         deadline = time.monotonic() + timeout
-        while True:
+        # Discarding an unhealthy session frees a ``_tracked`` slot, so
+        # ``_can_grow()`` flips straight back to True and the next iteration
+        # spawns again. A factory that always produces unhealthy instances
+        # (crash-on-launch browser, dead health-check endpoint) would
+        # otherwise loop forever, churning real browser processes. Bound the
+        # retries so the failure is fast and deterministic rather than a
+        # spin that only stops at the deadline.
+        budget = self._unhealthy_retry_budget()
+        for _ in range(budget):
             session = self._acquire_session(timeout, deadline)
-            if not self._is_healthy(session):
-                self._destroy(session)
-                continue
-            return session
+            if self._is_healthy(session):
+                return session
+            self._destroy(session)
+            if time.monotonic() >= deadline:
+                raise BrowserPoolError(
+                    f"no healthy session available within {timeout}s"
+                )
+        raise BrowserPoolError(
+            f"no healthy session after {budget} attempts; "
+            f"the factory or health_check is failing consistently"
+        )
+
+    def _unhealthy_retry_budget(self) -> int:
+        """Allow every pooled slot a couple of replacement attempts."""
+        return max(4, self._size * 3)
 
     def _acquire_session(self, timeout: float, deadline: float) -> PooledSession:
         try:

je_web_runner/utils/mock_services/servers.py

@@ -153,6 +153,8 @@ def do_POST(self):
                 server_state["issued"].append(token)
                 self._send(200, {
                     "access_token": token,
+                    # nosec B105 — OAuth token *type* constant; the token itself
+                    # above comes from secrets.token_hex.
                     "token_type": "Bearer",
                     "expires_in": 3600,
                 })

je_web_runner/utils/pagination_audit/audit.py

@@ -196,7 +196,7 @@
 
 # ---------- ordering check --------------------------------------------
 
 def assert_sorted_by(
     findings: PaginationFindings,
     items_by_page_key: KeyFn,
     *,
@@ -223,14 +223,21 @@
         return
     last = flattened[0]
     for current in flattened[1:]:
-        if reverse:
-            if current > last:
-                raise PaginationAuditError(
-                    f"order violation: {current!r} > {last!r} but reverse=True"
-                )
-        else:
-            if current < last:
-                raise PaginationAuditError(
-                    f"order violation: {current!r} < {last!r}"
-                )
+        # ``items_by_page_key`` is caller-supplied and only guaranteed to
+        # return Hashable, which is not necessarily orderable. Mixed types
+        # (str vs int, None vs anything) raise TypeError here — surface that
+        # as a PaginationAuditError instead of leaking a bare TypeError.
+        try:
+            out_of_order = current > last if reverse else current < last
+        except TypeError as error:
+            raise PaginationAuditError(
+                f"items_by_page_key returned non-comparable keys "
+                f"({current!r} vs {last!r}): {error}"
+            ) from error
+        if out_of_order:
+            symbol = ">" if reverse else "<"
+            suffix = " but reverse=True" if reverse else ""
+            raise PaginationAuditError(
+                f"order violation: {current!r} {symbol} {last!r}{suffix}"
+            )
         last = current

je_web_runner/utils/process_supervisor/supervisor.py

@@ -51,6 +51,11 @@ class OrphanFinding:
 ProcessLister = Callable[[], list[OrphanFinding]]
 ProcessKiller = Callable[[int], bool]
 
+# Every external process call is bounded: a wedged ``ps`` / ``tasklist`` /
+# ``taskkill`` would otherwise block the calling thread forever, which
+# defeats the point of a supervisor meant to unstick hung runs.
+_PROCESS_CALL_TIMEOUT_SECONDS = 15.0
+
 
 def _ps_unix_lister() -> list[OrphanFinding]:
     try:
@@ -59,8 +64,9 @@ def _ps_unix_lister() -> list[OrphanFinding]:
             ["ps", "-Ao", "pid=,comm=,args="],
             text=True,
             stderr=subprocess.DEVNULL,
+            timeout=_PROCESS_CALL_TIMEOUT_SECONDS,
         )
-    except (FileNotFoundError, subprocess.CalledProcessError) as error:
+    except (OSError, subprocess.SubprocessError) as error:
         raise ProcessSupervisorError(f"ps failed: {error!r}") from error
     findings: list[OrphanFinding] = []
     for line in out.splitlines():
@@ -84,8 +90,9 @@ def _tasklist_windows_lister() -> list[OrphanFinding]:
             ["tasklist", "/FO", "CSV", "/NH"],
             text=True,
             stderr=subprocess.DEVNULL,
+            timeout=_PROCESS_CALL_TIMEOUT_SECONDS,
         )
-    except (FileNotFoundError, subprocess.CalledProcessError) as error:
+    except (OSError, subprocess.SubprocessError) as error:
         raise ProcessSupervisorError(f"tasklist failed: {error!r}") from error
     findings: list[OrphanFinding] = []
     for line in out.splitlines():
@@ -116,14 +123,15 @@ def default_killer(pid: int) -> bool:
                 ["taskkill", "/F", "/PID", str(pid)],
                 stdout=subprocess.DEVNULL,
                 stderr=subprocess.DEVNULL,
+                timeout=_PROCESS_CALL_TIMEOUT_SECONDS,
             )
             return True
         # The PID list is filtered by ``KNOWN_DRIVER_NAMES`` and excludes
         # ``os.getpid()`` upstream, so this signal-9 only ever lands on the
         # supervisor's own webdriver children.
         os.kill(pid, 9)  # NOSONAR S4828 — pid pre-validated against driver name allow-list
         return True
-    except (OSError, subprocess.CalledProcessError) as error:
+    except (OSError, subprocess.SubprocessError) as error:
         web_runner_logger.warning(f"process_supervisor kill {pid} failed: {error!r}")
         return False
 

je_web_runner/utils/sharding/diff_shard.py

@@ -21,6 +21,8 @@ class DiffShardError(WebRunnerException):
 
 GitRunner = Callable[[Sequence[str]], str]
 
+_GIT_TIMEOUT_SECONDS = 30.0
+
 
 def _default_git_runner(args: Sequence[str]) -> str:
     cmd = ["git", *args]
@@ -30,8 +32,11 @@ def _default_git_runner(args: Sequence[str]) -> str:
             cmd,
             stderr=subprocess.DEVNULL,
             text=True,
+            # Bounded so a git subprocess waiting on an index lock or a
+            # credential prompt can't wedge the shard split indefinitely.
+            timeout=_GIT_TIMEOUT_SECONDS,
         )
-    except (FileNotFoundError, subprocess.CalledProcessError) as error:
+    except (OSError, subprocess.SubprocessError) as error:
         raise DiffShardError(f"git command failed: {error!r}") from error
     return out