[Bro-Dev] Rethinking Broker's blocking API

[Matthias Vallentin]

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

Agreed.

>     auto msg = ep.receive();
> 
>     if (msg)
>         return f(*msg); // unbox contained message
> 
>     if (msg.failed())
>         cout << "Trouble: " << to_string(msg.status()) << endl;
>     else
>         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:

  class error {
    uint8_t code();
    atom_value category();
    const message& context();
  };

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:

  // -- Usage -------------------------------------------

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

  if (msg) {
    f(*msg); // unbox message
  } else if (is_error(msg)) {
    cout << "error: " << to_string(msg.error()) << endl;
    if (msg == error::type_clash)
      // dispatch concrete error
  } else if (is_status(msg)) {
    cout << "status: " << to_string(msg.error()) << endl;
    if (msg == status::peer_added)
      // dispatch concrete status
  } else {
    // CAF error
  }

  // -- Broker implementation ---------------------------

  using failure = caf::error;

  enum status : uint8_t { /* define status codes * /};

  enum error : uint8_t { /* define error codes */ };

  bool is_error(const failure& f) { 
    return f.category() == atom("broker-error");
  }

  bool is_status(const failure& f) { 
    return f.category() == atom("broker-status");
  }

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

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

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:

  // -- Usage -------------------------------------------

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

  if (msg) {
    f(*msg); // unbox T
  } else if (auto e = msg.error()) {
    cout << "error: " << to_string(*e) << endl;
    if (*e == error::type_clash)
      // dispatch concrete error
  } else if (auto s = msg.status()) {
    cout << "status: " << to_string(*s) << endl;
    if (*s == status::peer_added)
      // dispatch concrete status
  } else {
    assert(!"not possible");
  }

  // -- Broker implementation ---------------------------

  enum status : uint8_t { /* define status codes * /};

  enum error : uint8_t { /* define error codes */ };

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

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

  private:
    expected<T> x_;
  };

Thoughts?

    Matthias