Skip to content

Commit d9ade50

Browse files
fix(cli): surface a rejected result and make IPC teardown non-raising
Two teardown-path defects on the job-api channel. SetResult's return value was discarded, but the handler returns false when it cannot route the job (unknown job key, or the executor threw). A rejected result was therefore treated as delivered and silently lost; it is now reported. The report reaches the original stderr rather than the channel that just rejected it. loop.close() also ran unconditionally after a timeout-bounded join. A wedged IPC thread leaves the loop running, so close() raised: inside the context manager's finally that masked the job's real exception (or turned a success into exit 1), and on the connect-failure path it pre-empted the bare raise, masking the original error. Teardown now lives in one non-raising helper used by both paths, which only closes the loop once its thread is gone.
1 parent fab5a28 commit d9ade50

2 files changed

Lines changed: 89 additions & 7 deletions

File tree

‎packages/uipath/src/uipath/_cli/_job_api.py‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,10 @@ def _result_sink(result: Any, output_arguments_file_path: str) -> None:
228228
future = asyncio.run_coroutine_threadsafe(
229229
callback.SetResult(job_id, dto), loop
230230
)
231-
future.result(timeout=_SET_RESULT_TIMEOUT_S)
231+
if future.result(timeout=_SET_RESULT_TIMEOUT_S) is False:
232+
logger.error(
233+
"The handler rejected the job result (SetResult returned false)"
234+
)
232235
except Exception:
233236
# Best-effort — a dropped result must be logged, not swallowed.
234237
logger.exception("Failed to deliver job result over IPC (SetResult)")
@@ -248,6 +251,24 @@ def clear_runtime_sinks() -> None:
248251
set_result_sink(None)
249252

250253

254+
def _stop_loop_thread(
255+
loop: asyncio.AbstractEventLoop, thread: threading.Thread, timeout: float
256+
) -> None:
257+
# Teardown must never raise: a wedged thread leaves the loop running, and close() would then
258+
# raise and mask the job's own outcome (or the error that got us here).
259+
try:
260+
loop.call_soon_threadsafe(loop.stop)
261+
except Exception:
262+
pass
263+
thread.join(timeout=timeout)
264+
if thread.is_alive():
265+
return
266+
try:
267+
loop.close()
268+
except Exception:
269+
pass
270+
271+
251272
def _new_ipc_event_loop() -> asyncio.AbstractEventLoop:
252273
"""A fresh event loop for the connection's thread (Proactor on Windows, required for named pipes)."""
253274
if sys.platform == "win32":
@@ -282,9 +303,7 @@ def _shutdown(self) -> None:
282303
)
283304
except Exception:
284305
pass
285-
self._loop.call_soon_threadsafe(self._loop.stop)
286-
self._thread.join(timeout=_SET_RESULT_TIMEOUT_S)
287-
self._loop.close()
306+
_stop_loop_thread(self._loop, self._thread, _SET_RESULT_TIMEOUT_S)
288307

289308

290309
def connect_handler_ipc(pipe: str, job_id: str) -> _HandlerIpcConnection:
@@ -313,9 +332,7 @@ async def _build() -> Any:
313332
)
314333
except BaseException:
315334
# On failure, don't leak the loop/thread.
316-
loop.call_soon_threadsafe(loop.stop)
317-
thread.join(timeout=_SET_RESULT_TIMEOUT_S)
318-
loop.close()
335+
_stop_loop_thread(loop, thread, _SET_RESULT_TIMEOUT_S)
319336
raise
320337
# Install in the caller's context, not on the IPC thread: the sinks are contextvars, resolved in
321338
# the job's own context at teardown. The ack still runs on `loop` (a separate thread) — no deadlock.

‎packages/uipath/tests/cli/test_job_api.py‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,71 @@ async def SendLog(self, jid: str, dto: Any) -> None:
211211
assert buf.getvalue().count("internal chatter") == 2
212212

213213

214+
def test_rejected_result_is_reported(monkeypatch):
215+
captured = _fake_output_sinks(monkeypatch)
216+
217+
class _Callback:
218+
async def SetResult(self, jid: str, dto: Any) -> bool:
219+
return False
220+
221+
class _Result:
222+
status = UiPathRuntimeStatus.SUCCESSFUL
223+
error = None
224+
225+
records: list[logging.LogRecord] = []
226+
227+
class _Capture(logging.Handler):
228+
def emit(self, record: logging.LogRecord) -> None:
229+
records.append(record)
230+
231+
api_logger = logging.getLogger("uipath._cli._job_api")
232+
handler = _Capture()
233+
api_logger.addHandler(handler)
234+
prior_level = api_logger.level
235+
api_logger.setLevel(logging.ERROR)
236+
237+
loop = asyncio.new_event_loop()
238+
thread = threading.Thread(target=loop.run_forever, daemon=True)
239+
thread.start()
240+
try:
241+
_job_api.install_runtime_sinks("job-8", _Callback(), loop)
242+
captured["sink"](_Result(), "out.args")
243+
finally:
244+
loop.call_soon_threadsafe(loop.stop)
245+
thread.join(timeout=5)
246+
loop.close()
247+
api_logger.removeHandler(handler)
248+
api_logger.setLevel(prior_level)
249+
250+
assert any("rejected" in r.getMessage().lower() for r in records)
251+
252+
253+
def test_teardown_does_not_raise_when_the_loop_thread_is_wedged():
254+
loop = asyncio.new_event_loop()
255+
started = threading.Event()
256+
release = threading.Event()
257+
258+
def _run() -> None:
259+
asyncio.set_event_loop(loop)
260+
261+
async def _block() -> None:
262+
started.set()
263+
release.wait(timeout=10)
264+
265+
loop.run_until_complete(_block())
266+
267+
thread = threading.Thread(target=_run, daemon=True)
268+
thread.start()
269+
assert started.wait(timeout=5)
270+
271+
_job_api._stop_loop_thread(loop, thread, 0.2)
272+
assert thread.is_alive()
273+
274+
release.set()
275+
thread.join(timeout=5)
276+
loop.close()
277+
278+
214279
def test_pending_log_sends_are_flushed_before_teardown(monkeypatch):
215280
_fake_output_sinks(monkeypatch)
216281
landed: list[str] = []

0 commit comments

Comments
 (0)