client http_request() response type

[Tectu]

@0x00002a I've been thinking about something today while on the road (so didn't check actual code): If I remember correctly, the current way that client::controller::http_request() and the corresponding connections are implemented does not allow defining a callback which accepts malloy::http::response right?

[0x00002a]

Well... sort of. The callback type is a visitor over malloy::http::reponse<T>... where T... is the body types returned by body_for (contained in a variant). This means that if the variant returned by body_for is a single-element one (i.e only one body type), such as with the default response filter, then the callback can indeed have the signature (malloy::http::response<...>) -> any but otherwise it needs to be templated over the body type (i.e. template<typename T>(malloy::http::response<T>) -> any).

Really that needs to be expressed as a concept, I'll look into it.

[Tectu]

Yeah, that's pretty ugly. I'll deem this important enough to tie it into the v0.1 release. Looking forward seeing your concepts :p

[0x00002a]

Yeah, that's pretty ugly

Hmm, ugly how? I agree the implementation details are rather messy but I went with it as a visitor because I think it provides a nice API. If only one type of response is needed, things are simple, if multiple are needed then the function becomes templated (or std::overloaded if that ever becomes standardised/someone writes one) and the user can decide what to do based on the body type themselves.

[Tectu]

Sorry, I've been unclear - doing way too many things today... 🙈

I meant to express that I am pro adding a proper concept for type restriction - the resulting error messages are "pretty ugly" when no concepts are in place.

[Tectu]

I have to get back to this - currently upgrading "old" application code to the latest main branch of malloy.

Given the following example:

struct foo
{
    void
    from_response(const malloy::http::response<>& http_resp);
};

std::future<malloy::error_code>
func()
{
    malloy::http::request req(
        malloy::http::method::get,
        m_host,
        m_port,
        "/system/plugins/available"
    );

    return m_malloy_controller->http_request(req, [h = std::move(handler)](auto&& resp) {
        foo d;
        d.from_response(resp);

        // ...
    });
}

I run into compilation errors:

error: cannot convert 'boost::beast::http::message<false, boost::beast::http::basic_string_body<char>, boost::beast::http::basic_fields<std::allocator<char> > >' to 'const malloy::http::response<>&'
   38 |         d.from_response(resp);
      |                         ^~~~
      |                         |
      |                         boost::beast::http::message<false, boost::beast::http::basic_string_body<char>, boost::beast::http::basic_fields<std::allocator<char> > >

So, what is going on here? malloy::http::response<> defaults the body type to string_body and the fields type to basic_fields<std::allocator<char>> - so how can it not convert the resulting boost::beast::http::message to malloy::http::response given that the types are the same? Does this boil down to a missing ctor in malloy::http::response to construct an object from an underlying boost::beast::measage?

I'm really sorry if I am missing something obvious here - not having the best day 🙈. Previously my applications consuming malloy were all built around the response being returned as an std::future<>. I have to upgrade to the new API now.

[0x00002a]

Its a combination of an oversight on my part and the lack of a ctor. The type passed to the caller should be malloy::http::response, but its beast::http::response instead (taken directly from the parser). I'll patch it along with the missing concept tonight or tomorrow. nvm you beat me too it :p

[Tectu]

[...] but its beast::http::response instead

I take it that that still needs fixing?