Replies: 1 comment 1 reply
-
The only one here I disagree with is the error handler. I think errors should just be ordinary messages, and not system messages, especially in the context of #1584. However, I base this only off of my experience working in a codebase where all actors are required to be typed actors, and all requests must have an explicit error handler. I think the following should apply (including the exception handler, which isn't included in the initial post): // The exit handler is a system message handler.
self->set_exit_handler([self](const caf::exit_msg& msg) {
// exit handler
self->quit(msg.reason);
}); // The exception handler is a system message handler.
self->set_exception_handler([self](std::exception_ptr err) {
try {
std::rethrow_exception(err);
} catch (...) {
// exception handler
}
}); // Down handlers are system message handlers.
self->monitor(hdl, [self](const caf::error& err) {
// down handler
}); // All message handlers defined in the behavior are ordinary message handlers.
auto my_actor(event_based_actor* self) -> behavior {
return {
[self](...) {
// user-defined handlers
},
[self](const caf::error& err) {
// error handler
},
[self](const caf::message& msg) {
// catch-all handler
},
};
} // Request response handlers are also just ordinary message handlers.
self->request(...).then(
[self](...) {
// result handler
},
[self](const caf::error& err) {
// error handler
}); There is actually one further special handler that currently is in a really bad place, which is // Idle handlers are system message handlers.
self->set_idle_handler(timeout, [self] {
// idle handler
}); |
Beta Was this translation helpful? Give feedback.
-
Starting this as a discussion here to have big-picture discussion instead of multiple discussions on details in tickets.
Not all messages are created equal. CAF has a few message types that bypass the regular actor behavior. In some cases (like streaming), this is fully transparent to the user. In other cases, users can still override the default processing. For this, CAF currently has a couple of special handlers. Namely:
default_handler
for catching messages that aren't matched by the behaviorerror_handler
for processingerror
messagesdown_handler
for processingdown_msg
messagesnode_down_handler
for processingnode_down_msg
messagesexit_handler
for processingexit_msg
messagesI think it's time to revisit some of the design decisions that lead to this implementation. Our toolkit in CAF is much richer than it has been in the past, especially due to
action
anddisposable
.As pointed out by @dominiklohmann (#1423), the
monitor
API is quite clumsy at the moment. Mostly for historic reasons, really. The current API based onmonitor
anddemonitor
withdown_msg
signaling goes back all the way to the beginning alongside the design forexit_msg
. However, down messages and exit messages are very different in nature. Down messages are explicitly requested by an user and users could just tell CAF what to do in case of the other actor going down at the point of callingmonitor
. Exit messages on the other hand can happen if another actors decides to link against an actor. Hence, basingmonitor
on the new actions seems much more natural. Instead of having a separatedemonitor
, users can simply calldispose
on the action and forget about it. With the current implementation, there's an inherent race when callingdemonitor
that sometimes will lead to receiving adown_msg
that is no longer relevant. So to cut a long story short, I think we can deprecatedown_handler
and simply use the suggested API from the ticket. I'd also consider the same API for node down messages.Regarding the default handler: we recently added a feature to CAF that allows a
behavior
to simply have a callback taking amessage
argument as a catch-all rule. The one "magic trick" that the default handler can pull off is stashing. I think this should receive it's own API instead. See #1474. Basically, instead of installing a skip handler, explicitly add a catch-all case and put messages to acaf::stash
(does not exist yet) that will allow flushing messages back to the mailbox whenever the user wants to. Doing it this way, it's much easier to understand what's going on. Users will also have a way of checking how full the stash actually is to make sure it's not exploding.The
error_handler
gets called forerror
messages. The most common way actors will receive errors is as a response to some sent message or when sending a message to an actor that drops it. The default is call quitquit(reason)
. Errors probably should stay separate. It's basically an "exception" case that shouldn't go through the regular behavior.Last but not least:
exit_handler
. My initial thought was to deprecate this as well and use the regular behavior. However, that would make the semantics of things likerequest(...).await(...)
a bit fuzzy. Usingawait
suspends the regular actor behavior. However, we always need to respond to system messages such asexit_msg
. So either we make an exception forexit_msg
and call the suspended behavior regardless or we have a special handler for it (status quo). The latter probably makes more sense, because we don't want to have a "default-case" handler from the regular behavior processing anexit_msg
. That would most likely result in tons of bugs resulting from discarding exit messages from default handlers. Same reasoning applies to theerror_handler
. So it's either keeping the special handlers for exit and error messages (my preference at the moment) or have them go to the regular behavior but exclude them from the default-case handler and call the behavior for these two types even if the behavior is suspended... sounds a bit convoluted to me.TLDR, I'm thinking of these changes to the API with CAF 1.0:
default_handler
=> deprecate; users should use[](const message&) { ... }
in their behavior to catch any regular message not covered by other handlers and use an explicitcaf::stash
class if they need to skip over certain messages to process them later ondown_handler
=> deprecate; replace withmonitor(handle, callback)
interfacenode_down_handler
=> deprecate; replace withmonitor(node, callback)
interfaceerror_handler
leave as-isexit_handler
leave as-isAs always, feedback most welcome. 🙂
Beta Was this translation helpful? Give feedback.
All reactions