[Bro-Dev] Rethinking Broker's blocking API

Matthias Vallentin vallentin at icir.org
Thu Jan 5 17:04:32 PST 2017


> 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:

    enum status_info : uint8_t {
      unknown_status = 0,
      peer_added,         ///< Successfully peered
      peer_removed,       ///< Successfully unpeered
      peer_incompatible,  ///< Version incompatibility
      peer_invalid,       ///< Referenced peer does not exist
      peer_unavailable,   ///< Remote peer not listening
      peer_lost,          ///< Lost connection to peer
      peer_recovered,     ///< Re-gained connection to peer
    };

And the error values:

    enum class ec : uint8_t {
      /// The unspecified default error code.
      unspecified = 1,
      /// Version mismatch during peering.
      version_incompatible,
      /// Master with given name already exist.
      master_exists,
      /// Master with given name does not exist.
      no_such_master,
      /// The given data store key does not exist.
      no_such_key,
      /// The operation expected a different type than provided
      type_clash,
      /// The data value cannot be used to carry out the desired operation.
      invalid_data,
      /// The storage backend failed to execute the operation.
      backend_failure,
    };

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

    expected<message> receive();

To be used as follows:

    auto msg = ep.receive();
    if (msg)
      return f(*msg); // unbox contained message
    switch (msg.error()) {
      default:
        cout << to_string(msg.error()) << endl;
        break;
      case status::peer_added:
        cout << "got new peer: " << msg.context() << endl;
        break;
      case status::peer_lost::
        break;
    }

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.

    Matthias


More information about the bro-dev mailing list