posit-dev / ark

Ark, an R kernel
MIT License
159 stars 9 forks source link

Ark: Panics in dummy kernel threads should be propagated to test harness #551

Open lionel- opened 3 days ago

lionel- commented 3 days ago

Currently, if a panic occurs in integration tests on a background thread involved in sending server-side zmq messages, the thread gets poisoned silently and the test harness hangs because the client is waiting for zmq messages that can no longer be sent. This makes it hard to debug issues both locally and on CI.

To fix that, we could use the same approach as for the Ark binary where we propagate panics to the main thread and then panic from there:

https://github.com/posit-dev/ark/blob/c6df0b2419dd3b5ee5a6a51bcef8d452b0245f9e/crates/ark/src/main.rs#L245-L249

lionel- commented 2 days ago

This approach doesn't really allow the panic to surface to the test harness. We only see an abnormal exit code without any panic message. Instead we should transmit the panic info to the dummy frontend thread and propagate from there.

Now the problem is that we can't currently receive messages with timeouts. We need to fix that using @DavisVaughan's approach in https://github.com/posit-dev/ark/pull/545/files/36357c43fc58a77641f4ef2a20d25bed3188a0fb..035ffcd9578a805152052b6e4f786c3a602dfeeb