Return types issues, working with JSON.

Sep 7, 2015 at 3:52 PM
Hello,

Return type issues seem very common to me and I would like to understand/fix these if possible. For example,
No viable conversion from 'task [...]' to 'task<[...]>'
Lets take a current example, this simple "getter" for a json array:
json::array EveCrest::getValues(const web::uri& address)
{
        http_client client(eveUriMain); // Create http_client to send the request.
//        uri_builder builder(address); // Build request URI.

        // Build custom JSON request as per
        // https://developers.eveonline.com/resource/crest
        http_request myReq(methods::GET);
        myReq.headers().set_content_type(eveSendContentType);
        myReq.set_request_uri(address);

        // Start request.
        return client.request(myReq)
        .then([=](http_response response) -> pplx::task<json::value>
        {
                if (response.status_code() != status_codes::OK) {
                        std::cout << "Couldn't GET data - "
                        << response.status_code() << std::endl;

                        return pplx::task_from_result(json::value());
                }

                if (!checkEveContentType(address, response)) {
                        std::cout << "Eve CREST didn't return correct Content-Type - "
                        << response.headers().content_type() << std::endl;

                        return pplx::task_from_result(json::value());
                }
                return response.extract_json(true);
        })

        .then([](pplx::task<json::value> task) -> json::array
        {
                try {
                        return task.get().as_object()[U("items")].as_array();
                } catch (const http_exception& e) {
                        std::cout << "Couldn't extract JSON - "
                        << e.what() << std::endl;
                }
        });
}
To me it seems ok... Return json::array on the last lambda. Pretty simple. I explicitely add the return types of each lambda since I find it easier to read. I still get the following error:
EveCrest.cpp:72:16: No viable conversion from 'typename details::_ContinuationTypeTraits<(lambda at EveCrest.cpp:91:15), value>::_TaskOfType' (aka 'task<typename _TaskTypeTraits<typename _FunctionTypeTraits<(lambda at EveCrest.cpp:91:15), value>::_FuncRetType>::_TaskRetType>') to 'json::array'
To be honest, these errors are quite cryptic and I am not sure what to make of it. Any help is very welcomed.
Sep 7, 2015 at 4:31 PM
OK, so this issue was my initial "return client.request()". From what I understand, by chaining tasks like this I need to return a pplx::task. Without the first return statement though, I am allowed to return what my last task needs to. Or am I understanding the documentation incorrectly?
Sep 21, 2015 at 9:48 PM
Ok, I edited your provided code to fix the two issues.

First, the result of your client.request().then().then() will be a pplx::task<json::array>, not a json::array. If you want your program to be more asynchronous, you can change the return type of getValues() to be a pplx::task<json::array>. If you're fine with it being synchronous, then you can call .get() on the task to change it into a value of json::array (this is what I did below).

Second, after that problem is fixed, I noticed you don't return or rethrow a value from your try/catch block in the final .then(). I assume you wanted to rethrow, so I threw a throw in there :).




Code:
    json::array getValues(const web::uri& address)
    {
        http_client client(U("http://www.microsoft.com/"));
        http_request myReq(methods::GET);
        myReq.headers().set_content_type(U("text/json"));
        myReq.set_request_uri(address);

        // Start request.
        pplx::task<json::array> task = client.request(myReq)
        .then([=](http_response response) -> pplx::task<json::value>
        {
            return response.extract_json(true);
        })
        .then([](pplx::task<json::value> task)->json::array
        {
            try {
                return task.get().as_object()[U("items")].as_array();
            }
            catch (const http_exception& e) {
                std::cout << "Couldn't extract JSON - "
                    << e.what() << std::endl;
                throw;
            }
        });
        return task.get();
    }
Sep 22, 2015 at 12:51 AM
Thank you for taking the time to help! Indeed, the problem was returning pplx::task. This was really my first foray into pplx (god it is great) and I didn't quite think about the ramifications of calling a "normal" method (blocking the software and thus making this whole thing moot).

On throwing and try/catch; it is just not a personal preference/style that I use. So I am fighting the lib a little bit on that one (especially at() throwing, which to me is quite extreme). That being said, so far it is working fine.

Here is what I have ended up implementing, making sure things are all asynchronous. Did I say pplx is awesome? :)
void EveCrest::init()
{
        getValues(eveMainUri)
        .then([this](pplx::task<json::value> myTask)
        {
                try {
                        json::object data = myTask.get().as_object();
                        parseUris(data);

                } catch (const json::json_exception& e) {
                        std::cout << "Couldn't parse JSON - "
                                << e.what() << std::endl;
                }
        })

        .then([this]()
        {
                // Start tasks in parallel.
                std::vector<pplx::task<void>> tasks = {
                        pplx::create_task([this] {
                                this->getItemNamesAndPrices(); })
                };

                // Wait for all tasks to finish.
                pplx::when_all(begin(tasks), end(tasks))
                .then([this]() {
                        std::cout << "CREST READY" << std::endl;
                        this->_isReady.store(true);
                }).wait();
        });
}
NB. There is only one task currently, eventually there will be many. getValues became:
pplx::task<json::value> EveCrest::getValues(const web::uri& address)
{
        http_client client(eveMainUri); // Will send the request.
        myReq.headers().set_content_type(eveSendContentType);
        myReq.set_request_uri(address);

        // Start request.
        return client.request(myReq)
        .then([=](http_response response) -> pplx::task<json::value>
              {
                        if (response.status_code() != status_codes::OK) {
                              std::cout << "Couldn't GET data - "
                                        << response.status_code() << std::endl;

                              return pplx::task_from_result(json::value());
                        }

                      return response.extract_json(true);
              });
}
I use an observer on the atomic isReady bool, notifying when the model is ready. Since I have other things to do while loading the app (pplx + yaml-cpp == love), things work out pretty darn well.
Sep 22, 2015 at 1:40 AM
Glad to know you're enjoying the library.

A few final comments on the above:

1) If you don't throw an exception from your task, all the continuations will get called. I strongly suspect this is not what you want to happen (I assume parseUris() is sneaking data through the this object and it's very important that it ends up being called). In the code above, this can be fixed by adding a throw; in the catch block.

2) I don't actually think your first .then() inside init() has any async operations inside it. You could probably merge it with the continuation below and make your code a bit cleaner.

3) PPLX has some built-in stuff for "eventing": check out http://microsoft.github.io/cpprestsdk/classpplx_1_1task__completion__event.html and https://msdn.microsoft.com/en-us/library/hh750136.aspx. Since you mentioned an atomic isReady bool, I'm assuming you're using a SpinLock-esque algorithm there. Generally, I've found that mixing cpu-blocking stuff like spinlocks with continuation based async yields ugly code that tries to make them "play nice". In the worst circumstances, you can actually end up with deadlocks!

For what I've seen of your code so far, it's probably ok. But I'd like to mention it in case you end up having deadlock issues in the future, so you know where to start looking.

4) Finally, most of your tasks don't do much exception handling. For your case, as I understand it, this is probably fine. However, if you find yourself wanting to improve the robustness of your async stuff (failure recovery, etc), note that you will need to trace and track down all the exception handling flows as well as the value flows. Feel free to come back and post another discussion if you run into issues (sometimes this part can be pretty tough).
Sep 22, 2015 at 9:24 AM
Thank you again for taking the time to thoroughly explain your observations and recommending improvements. There is a lot of new "material" to learn, so to speak, using task based c++. I am not confortable yet, that is what is so great about it. Even though I have my reservations on the JSON parsing in casablanca, pplx and the REST portion of the library have been like a breath of fresh air.

You are absolutely right about the first 2 observations. A lot of headaches avoided.

I wasn't fond of using a bool to notify my controllers, it seems very brittle. I will look into using eventing classes, that is probably more elegant than my current solution.

Finally, for the last point, I will keep that in mind when I start getting into more thorough exception handling. Right now I am just focused on getting the ground work done and working. The project is quite simple, so it seemed a great way to learn some new c++ fu.

Thanks again, all your explanations have been really helpful!