Skip to content

Commit 2c3fbd6

Browse files
dbrattliclaude
andauthored
fix: escalate supervisor-monitor failures instead of hanging (#36)
Closes #28. The monitor in `flatMapActorSupervised` is the top of its supervision tree — `Actor.spawn`, not `spawnLinked`, so nothing watches it in turn — and its loop body was bare. A throw from the user's `decide` callback, from `aobv.OnErrorAsync` on the Escalate path, or from the `unbox`/`Post` forward killed the loop outright. The stream then hung rather than failed: upstream kept calling `OnNextAsync`, the posts landed in a mailbox nobody read, and the observer saw neither OnCompleted nor OnError. On .NET the exception surfaced only on MailboxProcessor's unobserved Error event; on BEAM not at all. Guard the body, report the failure downstream, and stop looping — after an OnError the Rx grammar is closed, so there is nothing left to forward. The escalation path itself swallows secondary failures, since an exception thrown while reporting would reproduce the exact bug being fixed. The guard is a nested `actor { }` rather than `async { }` on purpose: it keeps `child <- ...` inside the actor process. Confirmed in the generated Erlang — the loop uses the actor builder's try_with and `child` stays process-dictionary backed via erlang:get/put. Test asserts the decider's own exception reaches downstream. Verified red-green: without the fix it fails on Quill's 5003ms timeout — the silent hang itself — and passes in 45ms with it. Adds `isOnError` to Tests.Utils. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ad64d16 commit 2c3fbd6

3 files changed

Lines changed: 90 additions & 19 deletions

File tree

‎src/ActorInterop.fs‎

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,17 @@ module internal ActorInterop =
4141
let emit value =
4242
dispatch.OnNextAsync value |> Async.Start'
4343

44+
// Terminating downstream must never itself throw: nothing supervises the
45+
// monitor, so an exception escaping here would kill it silently — the very
46+
// failure this is here to report.
47+
let escalate (ex: exn) =
48+
async {
49+
try
50+
do! aobv.OnErrorAsync ex
51+
with _ ->
52+
()
53+
}
54+
4455
// Monitor actor: spawns and supervises the child, swaps it on
4556
// Restart, and forwards upstream values to it. Routing values
4657
// through the monitor means the child is always set before the
@@ -61,25 +72,42 @@ module internal ActorInterop =
6172
actor {
6273
let! msg = inbox.Receive()
6374

64-
match Actor.tryAsChildExited msg with
65-
| Some exited ->
66-
let ex =
67-
match exited.Reason with
68-
| :? exn as e -> e
69-
| r -> ProcessExitException(sprintf "%A" r)
70-
71-
match decide ex with
72-
| Directive.Escalate -> do! aobv.OnErrorAsync ex
73-
| Directive.Stop -> ()
74-
| Directive.Restart -> child <- Actor.spawnLinked inbox (handler emit)
75-
// Anything that is not a ChildExited is an upstream value: both
76-
// backends deliver exits pre-converted (BEAM's receive_msg turns
77-
// trapped EXIT signals into ChildExited and drops `normal` ones,
78-
// .NET's spawnLinked posts one from its crash handler), so nothing
79-
// else can reach this mailbox.
80-
| None -> child.Post(unbox<'TSource> msg)
81-
82-
return! loop ()
75+
// The monitor is the top of this supervision tree — nothing
76+
// watches it in turn. Left bare, a throw from the user's `decide`,
77+
// from `OnErrorAsync`, or from the cast would kill the loop and
78+
// leave the subscription hanging with no terminal notification:
79+
// upstream keeps posting into a mailbox nobody reads. Report the
80+
// failure downstream and stop instead.
81+
let! alive =
82+
actor {
83+
try
84+
match Actor.tryAsChildExited msg with
85+
| Some exited ->
86+
let ex =
87+
match exited.Reason with
88+
| :? exn as e -> e
89+
| r -> ProcessExitException(sprintf "%A" r)
90+
91+
match decide ex with
92+
| Directive.Escalate -> do! escalate ex
93+
| Directive.Stop -> ()
94+
| Directive.Restart -> child <- Actor.spawnLinked inbox (handler emit)
95+
// Anything that is not a ChildExited is an upstream
96+
// value: both backends deliver exits pre-converted
97+
// (BEAM's receive_msg turns trapped EXIT signals into
98+
// ChildExited and drops `normal` ones, .NET's
99+
// spawnLinked posts one from its crash handler), so
100+
// nothing else can reach this mailbox.
101+
| None -> child.Post(unbox<'TSource> msg)
102+
103+
return true
104+
with ex ->
105+
do! escalate ex
106+
return false
107+
}
108+
109+
if alive then
110+
return! loop ()
83111
}
84112

85113
loop ())

‎test/ActorInteropTest.fs‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,44 @@ let tests =
208208
}
209209
)
210210

211+
testAsync (
212+
"flatMapActorSupervised escalates a throwing decider instead of hanging",
213+
async {
214+
// The decider is user code running inside the monitor loop. Before the monitor
215+
// caught its own failures, a throw here killed the supervisor silently and the
216+
// subscription hung forever with no terminal notification — this test would
217+
// then fail on Quill's timeout rather than on the assertion.
218+
let xs = fromNotification [ OnNext 1 ]
219+
let deciderError = System.InvalidOperationException("decider boom")
220+
221+
let result =
222+
xs
223+
|> Reactive.flatMapActorSupervised (fun _ -> raise deciderError) (fun emit inbox ->
224+
let rec loop () =
225+
actor {
226+
let! x = inbox.Receive()
227+
failwith "child boom"
228+
emit x
229+
return! loop ()
230+
}
231+
232+
loop ())
233+
234+
let obv = TestObserver<int>()
235+
let! _sub = result.SubscribeAsync obv
236+
do! obv.WaitUntil(List.exists isOnError)
237+
238+
// The decider's own failure is what reaches downstream, not the child's.
239+
let reported =
240+
obv.Notifications
241+
|> Seq.tryPick (function
242+
| OnError e -> Some e.Message
243+
| _ -> None)
244+
245+
assertThat reported (isEqualTo (Some "decider boom"))
246+
}
247+
)
248+
211249
testAsync (
212250
"flatMapActor forwards actor crash as OnError",
213251
async {

‎test/Utils.fs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ let isOnCompleted (n: Notification<'a>) =
3030
| OnCompleted -> true
3131
| _ -> false
3232

33+
let isOnError (n: Notification<'a>) =
34+
match n with
35+
| OnError _ -> true
36+
| _ -> false
37+
3338
/// Immutable snapshot of the observer state, returned across the process boundary.
3439
type private Snapshot<'a> =
3540
{ Notifications: Notification<'a> list

0 commit comments

Comments
 (0)