search

ncthbrt / reason-nact

let reason-nact = (node.js, reason, actors) ⇒ your µ services have never been so typed
https://nact.io
MIT License
126 stars 12 forks source link

Supervision policy callback doesn't get the message that was being processed at the time of crash #7

Closed yawaramin closed 6 years ago

yawaramin commented 6 years ago

In the documentation ( https://nact.io/lesson/reasonml/supervision ), it is mentioned that

A custom supervision policy is a function which takes in the exception which was thrown, the message which was being processed at the time at which the fault occurred, and the context of the parent.

However, in the Nact.rei interface, the supervision policy type is defined as

type supervisionPolicy('msg, 'parentMsg) =
  (exn, supervisionCtx('msg, 'parentMsg)) => Js.Promise.t(supervisionAction);

It looks like this callback will not get the message that was processed at time of crash. It would be really great to get that message in this callback function, because that would let me send back a meaningful error response from the supervision policy handler back to the calling actor.

You could introduce the message in a backwards-compatible way, as an optional parameter:

type supervisionPolicy('msg, 'parentMsg) =
  (~msg: 'msg=?, exn, supervisionCtx('msg, 'parentMsg)) => Js.Promise.t(supervisionAction);
ncthbrt commented 6 years ago

The tricky thing is that the parent doesn't know the message type of the faulted child (this allows much easier actor composition). Thus 'msg is the type of the parent message. It may instead be better to define the supervision policy on the child instead, this will likely require changes to the js, but you're right in that there are a number of use cases which make important to access the message.

My experience with ML is such that I might be missing an obvious avenue of correctly typing the supervision policy, so if you can think of an alternative approach, I'd love to hear it.

ncthbrt commented 6 years ago

@yawaramin I've discussed it with colleagues, and they agree that you have a good point 👍. In the next few days I'll modify the code to have the supervision policy specified on the child instead of the parent.

yawaramin commented 6 years ago

Supervision policy specified by the child itself is a good idea. It makes it possible for a top-level actor (i.e. a direct child of the actor system) to be supervised also. I haven't looked too much into Nact internals but this decoupled supervision approach will definitely be better. But I'll look through the internals a bit more and get back to you if anything stands out.

ncthbrt commented 6 years ago

@yawaramin v2.0.0 of ReasonNact is out and as promised allows you to specify the supervision policy on the child.

yawaramin commented 6 years ago

Amazing! I can't wait to try this out on my project :-)

yawaramin commented 6 years ago

https://github.com/ncthbrt/reason-nact/commit/146a40a8da4bb4006a33e51d078ca3f09a355731

yawaramin commented 6 years ago

Tried it out, works like a charm! I'll post some more details soon.