# Rethinking Broker's blocking API

**URL:** <https://community.zeek.org/t/rethinking-brokers-blocking-api/4572>\
**Category:** Development\
**Tags:** development\
**Created:** [January 2, 2017, 9:07pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572 "2017-01-02T21:07:21Z")\
**Posts on this page:** 15\
**Page:** 1

<div class="post-metadata">

**Author:** ![Matthias\_Vallentin1](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/matthias_vallentin1/32/596_2.png) [@Matthias\_Vallentin1](https://community.zeek.org/u/Matthias_Vallentin1)\
**Post date:** [January 2, 2017, 9:07pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/1 "2017-01-02T21:07:21Z")

</div>

Broker's current API to receive messages is as follows:

&nbsp;&nbsp;&nbsp;&nbsp;context ctx;  
&nbsp;&nbsp;&nbsp;&nbsp;auto ep = ctx.spawn\<blocking\>();  
&nbsp;&nbsp;&nbsp;&nbsp;ep.receive([&](const topic& t, const data& x) { .. });  
&nbsp;&nbsp;&nbsp;&nbsp;ep.receive([&](const status& s) { .. });

or the last two in one call:

&nbsp;&nbsp;&nbsp;&nbsp;ep.receive(  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[&](const topic& t, const data& x) { .. },  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[&](const status& s) { .. }  
&nbsp;&nbsp;&nbsp;&nbsp;);

The idea behind this API is that it's similar to the non-blocking  
endpoint API:

&nbsp;&nbsp;&nbsp;&nbsp;auto ep = ctx.spawn\<nonblocking\>();  
&nbsp;&nbsp;&nbsp;&nbsp;ep.subscribe([=](const topic& t, const data& x) { .. });  
&nbsp;&nbsp;&nbsp;&nbsp;ep.subscribe([=](const status& s) { .. });

Non-blocking endpoints should be the default, because they are more  
efficient due to the absence of blocking. For simplicity, the current  
API also provides a non-lambda overload of receive:

&nbsp;&nbsp;&nbsp;&nbsp;auto ep = ctx.spawn\<blocking\>();  
&nbsp;&nbsp;&nbsp;&nbsp;auto msg = ep.receive();  
&nbsp;&nbsp;&nbsp;&nbsp;std::cout \<\< msg.topic() \<\< " -\> " \<\< msg.data() \<\< std::endl;

Users can also check the mailbox of the blocking endpoint whether it  
contains a message:

&nbsp;&nbsp;&nbsp;&nbsp;// Only block if know that we have a message.  
&nbsp;&nbsp;&nbsp;&nbsp;if (!ep.mailbox().empty())  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;auto msg = ep.receive();

What I haven't considered up to now is the interaction of data and  
status messages in the blocking API. Both broker::message and  
broker::status are messages that linger the endpoint's mailbox. I find  
the terminology confusing, because a status instance is technically also  
a message. I'd rather speak of "data messages" and "status messages" as  
opposed to "messages" and "statuses". But more about the terminology  
later.

There's a problem with the snippet above. If the mailbox is non-empty  
because it contains a status message, the following call to receive()  
would hang, because it expects a data message. The only safe solution  
would be to use this form:

&nbsp;&nbsp;&nbsp;&nbsp;if (!ep.mailbox().empty())  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;ep.receive(  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[&](const topic& t, const data& x) { .. },  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[&](const status& s) { .. }  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;);

The problem lies in the receive() function that returns a message. It  
doesn't match the current architecture (a blocking endpoint has a single  
mailbox) and is not a safe API for users.

Here are some solutions I could think of:

&nbsp;&nbsp;&nbsp;&nbsp;(1) Let receive() return a variant\<message, status\> instead, because  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;the caller cannot know a priori what to expect. While simple to  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;call, it burdens the user with type-based dispatching afterwards.  
&nbsp;&nbsp;&nbsp;&nbsp;  
&nbsp;&nbsp;&nbsp;&nbsp;(2) Specify the type of message a user wants to receive, e.g.,

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;auto x = ep.receive\<message\>();  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;auto y = ep.receive\<status\>();

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;Here, don't like the terminology issues I mentioned above. More  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;reasonable could be

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;auto x = ep.receive\<data\>();  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;auto y = ep.receive\<status\>();

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;where x could have type data\_message with .topic() and .data(),  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;and y be a direct instance of type status.

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;But because callers don't know whether they'll receive a status  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;or data message, this solution is only an incremental  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;improvement.

&nbsp;&nbsp;&nbsp;&nbsp;(3) Equip blocking endpoints with separate mailboxes for data and  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;status messages. In combination with (2), this could lead to  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;something like:

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if (!ep.mailbox\<data\>().empty())  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;auto msg = ep.receive\<data\>();

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if (!ep.mailbox\<status\>().empty())  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;auto s = ep.receive\<status\>();

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;But now users have to keep track of two mailboxes, which is more  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;error-prone and verbose.

&nbsp;&nbsp;&nbsp;&nbsp;(4) Drop status messages unless the user explicitly asks for them.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;Do not consider them when working with an endpoint's mailbox,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;which only covers data messages.

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;While keeping the semantics of ep.receive() simple, it's not  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;clear how to poll for status messages. Should they just  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;accumulate in the endpoint and be queryable? E.g.,:

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// Bounded? Infinite?  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;const std::vector\<status\>& xs = ep.statuses();

Ultimately, I think it's important to have consistent API of blocking  
and non-blocking endpoints. Any thoughts on how to move forwards would  
be appreciated.

&nbsp;&nbsp;&nbsp;&nbsp;Matthias

---

<div class="post-metadata">

**Author:** ![robin](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/robin/32/599_2.png) [@robin](https://community.zeek.org/u/robin)\
**Post date:** [January 4, 2017, 8:48pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/2 "2017-01-04T20:48:50Z")

</div>

> Ultimately, I think it's important to have consistent API of blocking  
> and non-blocking endpoints. Any thoughts on how to move forwards would  
> be appreciated.

Nice summary of the challenge! I agree that none of the options you  
list sound really appealing. Here's an alternative idea: could we  
change your option 1 (the variant) into always returning \*both\*, i.e.,  
tuple\<status, message\>? To make that work, we'd add an additional  
(say) GOT\_MESSAGE status. Each time receive() gets called, it returns  
whatever's internally next on deck: either a message (with status set  
to GOT\_MESSAGE), or a "real" status (with the tuple's message set to  
null). The caller would then check the status first:

&nbsp;&nbsp;&nbsp;&nbsp;next = ev.receive()

&nbsp;&nbsp;&nbsp;&nbsp;if ( next.status() == GOT\_MESSAGE )  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;process\_message(next.msg());  
&nbsp;&nbsp;&nbsp;&nbsp;else {  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// Status/error handling here.  
&nbsp;&nbsp;&nbsp;&nbsp;}

This would actually align pretty nicely with how blocking APIs  
normally operate: returning potential errors directly with the call.  
And having the client code check for errors before using the result  
feels natural to me. (And it's actually same approach we discussed for  
the Bro-side API if we had tuples there. 🙂

What do you think?

Robin

---

<div class="post-metadata">

**Author:** ![Matthias\_Vallentin1](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/matthias_vallentin1/32/596_2.png) [@Matthias\_Vallentin1](https://community.zeek.org/u/Matthias_Vallentin1)\
**Post date:** [January 6, 2017, 1:04am UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/3 "2017-01-06T01:04:32Z")

</div>

> Nice summary of the challenge! I agree that none of the options you  
> list sound really appealing. Here's an alternative idea: could we  
> change your option 1 (the variant) into always returning \*both\*, i.e.,  
> tuple\<status, message\>?

You pushed me into a new direction. Broker already returns expected\<T\>  
for operations frequently (e.g., for blocking operations with data  
store) that yield either a T or a broker::error (which is just an alias  
for caf::error). How about we get rid of statuses entirely? Here are the  
current status enum values:

&nbsp;&nbsp;&nbsp;&nbsp;enum status\_info : uint8\_t {  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;unknown\_status = 0,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_added, ///\< Successfully peered  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_removed, ///\< Successfully unpeered  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_incompatible, ///\< Version incompatibility  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_invalid, ///\< Referenced peer does not exist  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_unavailable, ///\< Remote peer not listening  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_lost, ///\< Lost connection to peer  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_recovered, ///\< Re-gained connection to peer  
&nbsp;&nbsp;&nbsp;&nbsp;};

And the error values:

&nbsp;&nbsp;&nbsp;&nbsp;enum class ec : uint8\_t {  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// The unspecified default error code.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;unspecified = 1,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Version mismatch during peering.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;version\_incompatible,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Master with given name already exist.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;master\_exists,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Master with given name does not exist.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;no\_such\_master,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// The given data store key does not exist.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;no\_such\_key,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// The operation expected a different type than provided  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;type\_clash,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// The data value cannot be used to carry out the desired operation.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;invalid\_data,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// The storage backend failed to execute the operation.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;backend\_failure,  
&nbsp;&nbsp;&nbsp;&nbsp;};

Clearly, this could be merged together. This would yield a natural  
API for receive:

&nbsp;&nbsp;&nbsp;&nbsp;expected\<message\> receive();

To be used as follows:

&nbsp;&nbsp;&nbsp;&nbsp;auto msg = ep.receive();  
&nbsp;&nbsp;&nbsp;&nbsp;if (msg)  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;return f(\*msg); // unbox contained message  
&nbsp;&nbsp;&nbsp;&nbsp;switch (msg.error()) {  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;default:  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< to\_string(msg.error()) \<\< endl;  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;break;  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;case status::peer\_added:  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< "got new peer: " \<\< msg.context() \<\< endl;  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;break;  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;case status::peer\_lost::  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;break;  
&nbsp;&nbsp;&nbsp;&nbsp;}

This is pretty much what you suggested, Robin, just with a slight  
syntactical twist. The only downside I see is that "msg.error()" could  
be misleading, as we're sometimes not dealing with an error on the  
Broker framework level, just with respect to the call to  
expected\<message\>. That is, the error is that we didn't get a message,  
which we expected, but something that is a status change in the global  
topology, such as a new peer.

&nbsp;&nbsp;&nbsp;&nbsp;Matthias

---

<div class="post-metadata">

**Author:** ![robin](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/robin/32/599_2.png) [@robin](https://community.zeek.org/u/robin)\
**Post date:** [January 6, 2017, 1:16pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/4 "2017-01-06T13:16:38Z")

</div>

> &nbsp;&nbsp;&nbsp;&nbsp;expected\<message\> receive();

Yeah, I like that, except for one concern:

> switch (msg.error()) {  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;default:  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< to\_string(msg.error()) \<\< endl;  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;break;  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;case status::peer\_added:  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< "got new peer: " \<\< msg.context() \<\< endl;  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;break;  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;case status::peer\_lost::  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;break;  
> &nbsp;&nbsp;&nbsp;&nbsp;}

I think the name "error" is not just misleading but would also turn  
out tricky to use correctly. In that switch statement, if the default  
case is to handle only (real) errors, one would need to fully  
enumerate all the status:\* messages so that they don't arrive there  
too. More generally: the distinction between errors that signal  
trouble with the connection and expected state changes doesn't come  
through here.

What do you think about adding two methods instead of one to allow  
differentiating between status updates and errors explicitly: status()  
returns a joined result code like your error(); and failed() returns a  
boolean indicating if that status reflects an error situation:

&nbsp;&nbsp;&nbsp;&nbsp;auto msg = ep.receive();

&nbsp;&nbsp;&nbsp;&nbsp;if (msg)  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;return f(\*msg); // unbox contained message

&nbsp;&nbsp;&nbsp;&nbsp;if (msg.failed())  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< "Trouble: " \<\< to\_string(msg.status()) \<\< endl;  
&nbsp;&nbsp;&nbsp;&nbsp;else  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< "Status change: " \<\< to\_string(msg.status()) \<\< endl;

In either case one could then also use a switch to differentiate the  
status() further.

Robin

---

<div class="post-metadata">

**Author:** ![Matthias\_Vallentin1](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/matthias_vallentin1/32/596_2.png) [@Matthias\_Vallentin1](https://community.zeek.org/u/Matthias_Vallentin1)\
**Post date:** [January 6, 2017, 6:32pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/5 "2017-01-06T18:32:50Z")

</div>

> I think the name "error" is not just misleading but would also turn  
> out tricky to use correctly.

Agreed.

> &nbsp;&nbsp;&nbsp;&nbsp;auto msg = ep.receive();
> 
> &nbsp;&nbsp;&nbsp;&nbsp;if (msg)  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;return f(\*msg); // unbox contained message
> 
> &nbsp;&nbsp;&nbsp;&nbsp;if (msg.failed())  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< "Trouble: " \<\< to\_string(msg.status()) \<\< endl;  
> &nbsp;&nbsp;&nbsp;&nbsp;else  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< "Status change: " \<\< to\_string(msg.status()) \<\< endl;
> 
> In either case one could then also use a switch to differentiate the  
> status() further.

I think this is semantically what we want. Because broker::error is just  
a type alias for caf::error (and broker::expected just caf::expected),  
it's currently not possible to change the API of those classes. I see  
two solutions, one based on the existing vehicles and one that  
introduces a new structure with three states for T, error, and status.

Here's the first. A caf::error class has three interesting member  
functions:

&nbsp;&nbsp;class error {  
&nbsp;&nbsp;&nbsp;&nbsp;uint8\_t code();  
&nbsp;&nbsp;&nbsp;&nbsp;atom\_value category();  
&nbsp;&nbsp;&nbsp;&nbsp;const message& context();  
&nbsp;&nbsp;};

The member category() returns the type of error. In Broker, this is  
always "broker". CAF errors have "caf" as category. We could simply  
split the "broker" error category into "broker-status" and  
"broker-error" to distinguish the error class. We would keep the two  
different enums and provide free functions for a user-friendly API.  
Example:

&nbsp;&nbsp;// -- Usage -------------------------------------------

&nbsp;&nbsp;auto msg = ep.receive(); // expected\<message\>

&nbsp;&nbsp;if (msg) {  
&nbsp;&nbsp;&nbsp;&nbsp;f(\*msg); // unbox message  
&nbsp;&nbsp;} else if (is\_error(msg)) {  
&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< "error: " \<\< to\_string(msg.error()) \<\< endl;  
&nbsp;&nbsp;&nbsp;&nbsp;if (msg == error::type\_clash)  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// dispatch concrete error  
&nbsp;&nbsp;} else if (is\_status(msg)) {  
&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< "status: " \<\< to\_string(msg.error()) \<\< endl;  
&nbsp;&nbsp;&nbsp;&nbsp;if (msg == status::peer\_added)  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// dispatch concrete status  
&nbsp;&nbsp;} else {  
&nbsp;&nbsp;&nbsp;&nbsp;// CAF error  
&nbsp;&nbsp;}

&nbsp;&nbsp;// -- Broker implementation ---------------------------

&nbsp;&nbsp;using failure = caf::error;

&nbsp;&nbsp;enum status : uint8\_t { /\* define status codes \* /};

&nbsp;&nbsp;enum error : uint8\_t { /\* define error codes \*/ };

&nbsp;&nbsp;bool is\_error(const failure& f) {  
&nbsp;&nbsp;&nbsp;&nbsp;return f.category() == atom("broker-error");  
&nbsp;&nbsp;}

&nbsp;&nbsp;bool is\_status(const failure& f) {  
&nbsp;&nbsp;&nbsp;&nbsp;return f.category() == atom("broker-status");  
&nbsp;&nbsp;}

&nbsp;&nbsp;template \<class T\>  
&nbsp;&nbsp;bool failed(const expected\<T\>& x) {  
&nbsp;&nbsp;&nbsp;&nbsp;return !x && failed(x.error());  
&nbsp;&nbsp;}

&nbsp;&nbsp;template \<class T\>  
&nbsp;&nbsp;bool operator==(const expected\<T\>& x, status s) {  
&nbsp;&nbsp;&nbsp;&nbsp;return !x && x.error() == s;  
&nbsp;&nbsp;}

The only downside here is that we're still calling msg.error() in the  
case of a status. That's where the second option comes in. Let's call it  
result\<T\> for now (it won't matter much, because most people will use it  
with "auto"). A result\<T\> wraps an expected\<T\> and provides a nicer API  
to distinguish errors and status more cleanly. Example:

&nbsp;&nbsp;// -- Usage -------------------------------------------

&nbsp;&nbsp;auto msg = ep.receive(); // result\<message\>

&nbsp;&nbsp;if (msg) {  
&nbsp;&nbsp;&nbsp;&nbsp;f(\*msg); // unbox T  
&nbsp;&nbsp;} else if (auto e = msg.error()) {  
&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< "error: " \<\< to\_string(\*e) \<\< endl;  
&nbsp;&nbsp;&nbsp;&nbsp;if (\*e == error::type\_clash)  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// dispatch concrete error  
&nbsp;&nbsp;} else if (auto s = msg.status()) {  
&nbsp;&nbsp;&nbsp;&nbsp;cout \<\< "status: " \<\< to\_string(\*s) \<\< endl;  
&nbsp;&nbsp;&nbsp;&nbsp;if (\*s == status::peer\_added)  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// dispatch concrete status  
&nbsp;&nbsp;} else {  
&nbsp;&nbsp;&nbsp;&nbsp;assert(!"not possible");  
&nbsp;&nbsp;}

&nbsp;&nbsp;// -- Broker implementation ---------------------------

&nbsp;&nbsp;enum status : uint8\_t { /\* define status codes \* /};

&nbsp;&nbsp;enum error : uint8\_t { /\* define error codes \*/ };

&nbsp;&nbsp;template \<class T\>  
&nbsp;&nbsp;class result {  
&nbsp;&nbsp;public:  
&nbsp;&nbsp;&nbsp;&nbsp;optional\<caf::error\> status() const;  
&nbsp;&nbsp;&nbsp;&nbsp;optional\<caf::error\> error() const;

&nbsp;&nbsp;&nbsp;&nbsp;// Box semantics  
&nbsp;&nbsp;&nbsp;&nbsp;explicit operator bool() const;  
&nbsp;&nbsp;&nbsp;&nbsp;T& operator\*();  
&nbsp;&nbsp;&nbsp;&nbsp;const T& operator\*() const;  
&nbsp;&nbsp;&nbsp;&nbsp;T\* operator-\>();  
&nbsp;&nbsp;&nbsp;&nbsp;const T\* operator-\>() const;

&nbsp;&nbsp;private:  
&nbsp;&nbsp;&nbsp;&nbsp;expected\<T\> x\_;  
&nbsp;&nbsp;};

Thoughts?

&nbsp;&nbsp;&nbsp;&nbsp;Matthias

---

<div class="post-metadata">

**Author:** ![robin](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/robin/32/599_2.png) [@robin](https://community.zeek.org/u/robin)\
**Post date:** [January 6, 2017, 9:44pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/6 "2017-01-06T21:44:19Z")

</div>

I'm going back and forth between the two versions. I think I'd take  
the 2nd (the custom class), though maybe with the API I had used in my  
example instead (i.e, msg.failed() and then a single status() method  
for both cases) as having two methods error/status returning the same  
thing looks a bit odd. But not a big deal either way, any of these  
options sounds fine to me.

Robin

---

<div class="post-metadata">

**Author:** ![Matthias\_Vallentin1](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/matthias_vallentin1/32/596_2.png) [@Matthias\_Vallentin1](https://community.zeek.org/u/Matthias_Vallentin1)\
**Post date:** [January 9, 2017, 7:34pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/7 "2017-01-09T19:34:23Z")

</div>

> But not a big deal either way, any of these options sounds fine to me.

This is the synopsis for the slightly adapted message class, no other  
changes:

&nbsp;&nbsp;&nbsp;&nbsp;class message {  
&nbsp;&nbsp;&nbsp;&nbsp;public:  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Checks whether a message is a (topic, data) pair.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// @returns `true` iff the message contains a (topic, data) pair.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;explicit operator bool() const;

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// @returns the contained topic.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// @pre `static_cast\<bool\>(*this)`  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;const broker::topic& topic() const;

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// @returns the contained topic.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// @pre `static_cast\<bool\>(*this)`  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;const broker::data& data() const;

&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// @returns the contained status.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// @pre `!*this`  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;const broker::status& status() const;  
&nbsp;&nbsp;&nbsp;&nbsp;};

I opted against a .failed() member in broker::message, because it's up  
to the concrete status instance to define failure or a mere status  
change. A message is now either a (topic, data) pair or a status  
instance.

To distinguish between the two status, I use operator bool. Since this  
is almost the same as a failed() method, I have one last idea: instead  
of returning const-references to topic, data, and instances. We could  
simply return non-owning const-pointers, which are non-null if and only  
if the respective type is active:

&nbsp;&nbsp;&nbsp;&nbsp;if (auto data = msg.data())  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;f(\*data)  
&nbsp;&nbsp;&nbsp;&nbsp;else  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;g(\*msg.status())

I've also updated how to do status/error handling. See the documentation  
at [http://bro.github.io/broker/comm.html#status-and-error-handling](http://bro.github.io/broker/comm.html#status-and-error-handling) for  
details.

&nbsp;&nbsp;&nbsp;&nbsp;Matthias

---

<div class="post-metadata">

**Author:** ![robin](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/robin/32/599_2.png) [@robin](https://community.zeek.org/u/robin)\
**Post date:** [January 10, 2017, 2:21pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/8 "2017-01-10T14:21:54Z")

</div>

> To distinguish between the two status, I use operator bool.

I don't see that in the "status and error handling" section. Would I  
do "if (! status) { \<error stuff\> }"? That doesn't seem quite  
intuitive. I think a method with a descriptive name would be better  
here.

> &nbsp;&nbsp;&nbsp;&nbsp;if (auto data = msg.data())  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;f(\*data)  
> &nbsp;&nbsp;&nbsp;&nbsp;else  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;g(\*msg.status())

For this I think I prefer the boolean version: "if (msg) { \<use  
msg.data() })".

Robin

---

<div class="post-metadata">

**Author:** ![Matthias\_Vallentin1](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/matthias_vallentin1/32/596_2.png) [@Matthias\_Vallentin1](https://community.zeek.org/u/Matthias_Vallentin1)\
**Post date:** [January 10, 2017, 3:39pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/9 "2017-01-10T15:39:47Z")

</div>

> \> To distinguish between the two status, I use operator bool.
> 
> I don't see that in the "status and error handling" section. Would I  
> do "if (! status) { \<error stuff\> }"? That doesn't seem quite  
> intuitive. I think a method with a descriptive name would be better  
> here.

Sorry, that was misleading. Statuses don't provide an operator bool.  
They could, however, to distinguish them from errors.

> \> if (auto data = msg.data())  
> \> f(\*data)  
> \> else  
> \> g(\*msg.status())
> 
> For this I think I prefer the boolean version: "if (msg) { \<use  
> msg.data() })".

Okay, I'll switch that over. I prefer the other way, though :-).

&nbsp;&nbsp;&nbsp;&nbsp;Matthias

---

<div class="post-metadata">

**Author:** ![Matthias\_Vallentin1](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/matthias_vallentin1/32/596_2.png) [@Matthias\_Vallentin1](https://community.zeek.org/u/Matthias_Vallentin1)\
**Post date:** [January 10, 2017, 4:28pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/10 "2017-01-10T16:28:31Z")

</div>

> Sorry, that was misleading. Statuses don't provide an operator bool.  
> They could, however, to distinguish them from errors.

Mulling over this more, I'm not sure if it's clear which status codes  
constitute an error. For example, there's a peer\_lost and peer\_recovered  
status code. Is only the first an error? Some users may consider peer  
churn normal.

Here's the list of all status codes:

&nbsp;&nbsp;&nbsp;&nbsp;enum class sc : uint8\_t {  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// The unspecified default error code.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;unspecified = 1,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Successfully added a new peer.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_added,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Successfully removed a peer.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_removed,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Version incompatibility.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_incompatible,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Referenced peer does not exist.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_invalid,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Remote peer not listening.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_unavailable,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// An peering request timed out.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_timeout,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Lost connection to peer.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_lost,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Re-gained connection to peer.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_recovered,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Master with given name already exist.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;master\_exists,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// Master with given name does not exist.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;no\_such\_master,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// The given data store key does not exist.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;no\_such\_key,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// The store operation timed out.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;request\_timeout,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// The operation expected a different type than provided  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;type\_clash,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// The data value cannot be used to carry out the desired operation.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;invalid\_data,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/// The storage backend failed to execute the operation.  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;backend\_failure,  
&nbsp;&nbsp;&nbsp;&nbsp;};

If we provided operator bool() for statuses, then it would be true for  
peer\_added, peer\_removed\_, peer\_recovered, and false for all others.  
This selection seems arguable to me, which is why I'm inclined to let  
users probe for specific instances themselves.

&nbsp;&nbsp;&nbsp;&nbsp;Matthias

---

<div class="post-metadata">

**Author:** ![robin](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/robin/32/599_2.png) [@robin](https://community.zeek.org/u/robin)\
**Post date:** [January 10, 2017, 4:46pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/11 "2017-01-10T16:46:27Z")

</div>

I see the challenge but I think we need some way to differentiate  
serious errors from expected updates, otherwise we're back at writing  
switch statements that need to comprehensively list all cases. One can  
always post-filter if, e.g., one does consider status X not an error  
even though it's flagged as such. Instead of a binary error yes/no,  
what about levels along these lines: (1) Error: \*we\* did something  
seriously wrong; (2) Warning: something's seems off, including  
problems with peers; and (3) Info: just an update on activity. It's  
not clear-cut of course but it would still be good to have some  
default classification for cases that one doesn't handle directly (and  
if it's only for then logging as error/warning/info). One could then  
also compare the level directly with the status object: "if ( status  
== ERROR ) ..."

Robin

---

<div class="post-metadata">

**Author:** ![Matthias\_Vallentin1](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/matthias_vallentin1/32/596_2.png) [@Matthias\_Vallentin1](https://community.zeek.org/u/Matthias_Vallentin1)\
**Post date:** [January 10, 2017, 5:38pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/12 "2017-01-10T17:38:47Z")

</div>

> I see the challenge but I think we need some way to differentiate  
> serious errors from expected updates, otherwise we're back at writing  
> switch statements that need to comprehensively list all cases.

I agree that writing switch statements is not very productive. From a  
user perspective, it's important to distinguish whether I can ignore  
a status update, or whether I have to react to it. I would consider more  
fine-grained classifications arbitrary. In particular, I find 3 levels  
too complex. Users will wonder "wait, was this an info or a warning?"  
and then have to go back to the documentation.

How about making it explicit by providing two types of codes, status and  
errors:

&nbsp;&nbsp;&nbsp;&nbsp;enum class sc : uint8\_t {  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;unspecified = 1,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_added,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_removed,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_recovered,  
&nbsp;&nbsp;&nbsp;&nbsp;};

&nbsp;&nbsp;&nbsp;&nbsp;enum class ec : uint8\_t {  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;unspecified = 1,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_incompatible,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_invalid,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_unavailable,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_lost,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;peer\_timeout,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;master\_exists,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;no\_such\_master,  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;...  
&nbsp;&nbsp;&nbsp;&nbsp;};

This is close to the original design, except that the peer\_\* codes are  
now split into errors and status, and that we still have a single status  
class to combine them both:

&nbsp;&nbsp;&nbsp;&nbsp;auto s = msg.status();  
&nbsp;&nbsp;&nbsp;&nbsp;if (s.error())  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// Compare against error codes for details.  
&nbsp;&nbsp;&nbsp;&nbsp;else  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// Compare against status codes for details.

Or simpler: avoiding the explicit distinction of error and status codes  
and let the function status::error() return true for what we deem  
actionable errors.

&nbsp;&nbsp;&nbsp;&nbsp;Matthias

---

<div class="post-metadata">

**Author:** ![robin](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/robin/32/599_2.png) [@robin](https://community.zeek.org/u/robin)\
**Post date:** [January 10, 2017, 11:24pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/13 "2017-01-10T23:24:01Z")

</div>

> From a user perspective, it's important to distinguish whether I can  
> ignore a status update, or whether I have to react to it.

Yep, exactly.

> I would consider more fine-grained classifications arbitrary. In  
> particular, I find 3 levels too complex.

Well, weren't you just saying that two levels are arbitrary? 🙂 I  
tried to make it a bit less arbitrary by providing more options. Your  
new suggestion goes to back yes/no in terms of whether it's an error  
or not, which I think is ok, but you seemed concerned about that.

> auto s = msg.status();  
> &nbsp;&nbsp;&nbsp;if (s.error())  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// Compare against error codes for details.  
> &nbsp;&nbsp;&nbsp;else  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// Compare against status codes for details.

I like this (with the two separate types for error/status code, as  
that makes the distinction explicit).

Could we then now also lift the error() method into the message class?  
So "if (msg.error())" would be a shortcut for  
"if(msg.status().error()? (And then we'd back be where we started I  
believe. 🙂

Btw, there's one more complexity with all this: when one gets a  
status, one cannot tell which operation it belongs to, as it depends  
on how thing got queued and processed internally. That can make it  
hard to react to something more specifically. But that's something we  
can't avoid with the asynchronous processing.

Robin

---

<div class="post-metadata">

**Author:** ![Matthias\_Vallentin1](https://yyz1.discourse-cdn.com/flex011/user_avatar/community.zeek.org/matthias_vallentin1/32/596_2.png) [@Matthias\_Vallentin1](https://community.zeek.org/u/Matthias_Vallentin1)\
**Post date:** [January 11, 2017, 2:49am UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/14 "2017-01-11T02:49:21Z")

</div>

> Could we then now also lift the error() method into the message class?  
> So "if (msg.error())" would be a shortcut for  
> "if(msg.status().error()? (And then we'd back be where we started I  
> believe. 🙂

Done.

And yes, we've done the full circle. 😉 But at least we're in full  
agreement now. Sometimes converging takes a little longer.

> Btw, there's one more complexity with all this: when one gets a  
> status, one cannot tell which operation it belongs to, as it depends  
> on how thing got queued and processed internally. That can make it  
> hard to react to something more specifically. But that's something we  
> can't avoid with the asynchronous processing.

I think we can address this. First, the status code tells roughly what  
went wrong. Second, a status has a context\<T\>() method that returns  
additional information when available. For example, all peering-related  
statuses include a broker::endpoint\_info instance to figure out exactly  
what peer created the issue. If we need the fine granularity, all we  
need is to add the necessary context information. (This is enforced at  
compile time, by the way; for a given status code, the context is either  
a T or not present, but not of a different type.)

&nbsp;&nbsp;&nbsp;&nbsp;Matthias

---

<div class="post-metadata">

**Author:** ![system](https://canada1.discourse-cdn.com/flex011/uploads/zeek/original/1X/f09d732bc2cc7c7cc7e35db67cf4e1d5233ce7a7.png) [@system](https://community.zeek.org/u/system)\
**Post date:** [May 6, 2022, 3:44pm UTC](https://community.zeek.org/t/rethinking-brokers-blocking-api/4572/15 "2022-05-06T15:44:26Z")

</div>