Re: [cpp-netlib-devel] soliciting ideas concerning client errors

[Dean M. B. <mik...@gm...>]

On Wed, Jan 14, 2009 at 6:03 AM, John P. Feltz <jf...@ov...> wrote:
> Dean Michael Berris wrote:
>>
>> I agree that based on experience there aren't strictly 1.0 or 1.1
>> clients -- but the original intent of the library was to keep it
>> simple: so simple in fact that much of the nuances of the HTTP
>> protocol are hidden from the user. The aim is not to treat the users
>> as "stupid" but to give them a consistent and simple API from which
>> they can write their more important application logic around.
>>
>>
>
> I'm not suggesting getting rid of this simple API. rfc based policies
> are crucial as a foundation for defining client behavior at certain
> level. I'm suggesting that level to be at the implementation. A wrapper
> can be provided with certain predefined options, be those polices or
> non-policies to do what was the original design goal for the client class.
>

Ok.

How do you see this wrapper helping keep the API simple then?

>> I appreciate the idea of using policies, but the idea of the policies
>> as far as generic programming and design goes allows for well-defined
>> points of specialization. Let me try an example (untested):
>>
>> template <
>>   class Tag,
>>   class Policies,
>>   unsigned version_major,
>>   unsigned version_minor
>>
>> class basic_client : mpl:inherit_linearly<Policies>::type {
>>   private:
>>     typedef mpl:inherit_linearly<Policies>::type base;
>>   // ...
>>   basic_response<Tag> get(basic_request<Tag> const & request) {
>>     shared_ptr<socket> socket_ = base::init_connection(request);
>>     return base::perform_request(request, "GET");
>>   }
>> }
>> whatever implementation of init_connection is available to the
>> available policies. Of course this assumes that the policies used are
>> orthogonal. Design-wise it would be nice to enforce at compile-time
>> the concepts of the policies valid for the basic_client, but that's
>> for later. ;)
>>
>
> Of the necessary concerns, I see the need to provide one for supplying
> the resolved, the sockets, and logic governing both. These seem
> nonorthogonal, and so facading is more appropriate for right now.
> Additionally, I don't write code through the lens of a policy based
> strategy until I've seen what the implementation requires.
>

Great. So what about nested policies?

management_policy<resolver_policy, socket_policy>

Where you have many management policies:

persistent_management_policy
strict_http_1_1_management_policy
proxy_aware_http
...

And many resolver policies

async_resolver_policy
sync_resolver_policy
...

And socket creation policies

sync_connect_policy
async_connect_policy
timeout_policy
...

?

>> The lack of an existing HTTP Client that's "really 1.0" or "really
>> 1.1" is not an excuse to come up with one. ;-)
>>
>
> I'm sure users wishing to optimize requests to legacy servers or servers
> which do not follow a complete 1.1 spec will find that rather
> frustrating. Especially since it something so simple to design for (see
> branch).
>

True, but notice that HTTP 1.0 servers which return HTTP 505 errors
that says it doesn't support HTTP 1.1 should allow the user to use the
appropriate client. This logic should be provided by the user of the
library, instead of the library writers -- where our aim is to provide
a reasonably compliant *and* simple API.

This is similar to the discussion about providing a public method
'perform' with a string provided as the action and my stance doesn't
change -- the point of an HTTP client is to be reasonably compliant to
the HTTP 1.0/1.1 spec, and thus only supported or "standard" methods
are supported.

So having something like:

struct http_client_base {
  http_client_base() {}
  virtual http::response get(http::request const & request);
}

struct http_1_1_client : http_client_base {
  http_1_1_client() {}
  http::response get(http::request const & request) { return
client_.get(request); }
  private:
    basic_client<http::message_tag, 1, 1> client_;
}

struct http_1_0_client : http_client_base {
  http_1_0_client() {}
  http::response get(http::request const & request) { return
client_.get(request); }
  private:
    basic_client<http::message_tag, 1, 0> client_;
}

That's written by the user of the library to support his use case of a
dynamically determined HTTP 1.0 and HTTP 1.1 client would not be
entirely impossible. OTOH, the responsibility of
basic_client<http::message_tag, 1, 0> would be to provide a
sufficiently compliant HTTP 1.0 client, and
basic_client<http::message_tag, 1, 1> would be to provide a
sufficiently compliant HTTP 1.1 client.

>> So what I'm saying really is, we have a chance (since this is
>> basically from scratch anyway) to be able to do something that others
>> have avoided: writing a simple HTTP client which works when you say
>> it's really 1.0, or that it's really 1.1 -- in the simplest way
>> possible. The reason they're templates are that if you don't like how
>> they're implemented, then you can specialize them to your liking. :-)
>>
>
> This is a fine goal, however assuming that the simplest design will
> address what I believe to be all the complex concerns is flawed. Let us
> solve the complex issues _first_, then simplify. There is nothing wrong
> with the basic_clients API in principle, though I think this should
> address complex users needs first and so delegation of that class to
> single request processing + a wrapper incorporating certain policies
> with predefined behavior is the best course for now.
>

I still don't see how complex really the concerns are.

If you're worried about unrecoverable errors, then you have exceptions
-- which is perfectly fine. I would even go farther and say we should
have special exception types to denote different types of exceptions
all deriving from std::runtime_error.

If you're worried about unrecoverable errors *and* you don't want
exceptions, you'll use the signature I've shown below. I might even go
and change the signature from

  tuple<basic_response<Tag>, error_code> get(basic_request<Tag> const
& request, no_throw_t(*)());

to something like

  tuple<optional<basic_response<Tag> >, optional<error_code> >
get(basic_request<Tag> const & request, no_throw_t(*)());

And enforce that either the response is not defined if error_code is
defined, or that basic_response<Tag> was defined if error_code is not
defined. I might even explore the possibility of using a
boost::variant, but I'm not too comfortable with the usage semantics
of variants. This might require some more thought on my part, but I'm
thinking aloud on how I'll address the conveyance of errors from the
client side.

>>
>> Actually when it comes to error-handling efficiency (i.e. avoiding
>> exceptions which I think are perfectly reasonable to have), I would
>> have been happy with something like this:
>>
>> template <...>
>> class basic_client {
>>   basic_response<Tag> get(basic_request<Tag> const & request);
>>   tuple<basic_response<Tag>, error_code> get(basic_request<Tag> const
>> & request, no_throw_t(*)());
>> }
>>
>> This way, if you call 'get' with the nothrow function pointer
>> argument, then you've got yourself covered -- and instead of a
>> throwing implementation, the client can return a pair containing the
>> (possibly empty) basic_response<Tag> and an (possibly
>> default-constructed) error_code.
>>
>
> I don't see where function pointers belong in a client which intends to
> remain simple. I'm also against a no-throw parameter because it is less
> explicit. If an exception is thrown then the user knows there's an issue
> and a valid response was not returned. If a response is returned either
> way, than it is easier for the user to miss.
>

This is how you use that signature (with the unused function pointer):

http::client client_;
http::request request("http://www.booost.org");
http::response response;
boost::system::error_code error_code;
tie(response, error_code) = client_.get(request, http::nothrow);

The http::nothrow function would be defined as such:

struct no_throw_t;

inline no_throw_t no_throw() {
  return no_throw_t();
}

struct no_throw_t {
  private:
    no_throw_t();
    friend no_throw_t no_throw();
};

:-)

>>
>> Actually, there's nothing in the HTTP 1.0 spec that says a response
>> that's HTTP 1.x where x != 0 is an invalid response. There's also
>> nothing in the spec that says that HTTP 1.1 requests cannot be
>> completed when the HTTP 1.0 response is received.
>>
>
> These are cases that I'm not concerned with.
>

Okay, but if you were concerned with compliance when it comes to
things like persistent connections, request pipelining support, and
streaming encodings, maybe making them specializations of the basic
client using a different tag on different HTTP version numbers would
be something you'd like to explore. :-)

>>> I have no comment regarding a-sync usage as I've not looked into that
>>> issue in depth, I've only tried to make existing policy functions as
>>> re-usable as possible for that case.
>>>
>>
>> Which I think is the wrong approach if you ask me.
>>
>>
> By existing I was referring to things like stateless items such as
> append_* in the branch. If these can't be used asynchronously I would be
> curious as to why. I've expended no other effort in regards to this.

Ok. :)


>>
>> I think if you really want to be able to decompose the client into
>> multiple orthogonal policies, you might want to look into introducing
>> more specific extension points in the code instead of being hampered
>> by earlier (internal) design decisions. ;)
>>
>>
>
> I am growing tired of discussing this and would prefer to just get on
> with implementing _something_ in the branch. After tomorrow I'll be
> going back to my day job and time for working on this will be limited
> and I don't want to spend that time debating design decisions in which
> there can be very little compromise.
>

Okay.

At any rate, since the basic_client is meant to be the "world-facing"
API, it's not entirely impossible to write auxiliary classes that were
used by a specialization of the basic_client with a different tag.
What I mean by this is that for example, you can have a different tag
and a different set of (new) traits and accessors from which you can
design the basic_client specialization around.

struct non_throwing_tag;
struct persistent_connections_tag;

namespace triats {
  template <class T>
  struct http_tag;

  template <>
  struct http_tag<non_throwing_tag> {
    typedef http::message_tag type;
  };

  template <>
  struct http_tag<persistent_connections_tag> {
    typedef http::message_tag type;
  };
}

template<>
class basic_client<non_throwing_tag, 1, 0> {
  // different API of a client that doesn't throw on errors
}

template<>
class basic_client<persistent_connections_tag, 1, 0> {
  // implement a HTTP 1.0 client that supports persistent connections
}

Internally in all these specializations, you can pretty much do
whatever you want. ;-)

Then from there (and this approach) we can essentially write unit
tests that make sure the implementations of the specifializations do
what they're meant to do, and then we can look into how we can merge
the current implementation of the basic_client to accomodate or even
leverage the work you've done into the trunk. Maybe that would then
get us release 0.4 having an HTTP 1.1 client available. :D

How does that sound to you?

-- 
Dean Michael C. Berris
Software Engineer, Friendster, Inc.