2

I am using the poco libraries, and I'm trying to wrap them into a nicer HTTPClient that I can use everywhere in a few lines, and also make them async. To that end, I am using std::future and a custom response struct. However, for some reason, it tells me it's trying to reference a deleted function. I'm not deleting anything, so I don't really know why it would be saying this.

httpclient.h

#include <Poco/Net/HTTPSClientSession.h>
#include <Poco/Net/HTTPRequest.h>
#include <Poco/Net/HTTPResponse.h>
#include <Poco/Exception.h>
#include <Poco/URI.h>
#include <future>
#include <map>

typedef struct response {
    std::istream& is;
    Poco::Net::HTTPResponse& response;
} RES;

class HttpClient {
public:
    static std::future<RES> get(Poco::Net::HTTPSClientSession& session, Poco::URI url, std::map<std::string, std::string> headers = {});
};

httpclient.cpp

#include "httpclient.h"

std::future<RES> HttpClient::get(Poco::Net::HTTPSClientSession& session, Poco::URI url, std::map<std::string, std::string> headers) {
    return std::async(std::launch::async, [&session, url, headers](){
        try {
            std::string path(url.getPathAndQuery());
            if (path.empty()) path = "/";

            Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, path, Poco::Net::HTTPMessage::HTTP_1_1);
            request.add("Content-Length", "0");

            if (headers.size() > 0) {
                for (std::map<std::string, std::string>::const_iterator itr = headers.begin(); itr != headers.end(); ++itr) {
                    request.add(itr->first, itr->second);
                }
            }

            Poco::Net::HTTPResponse _res;
            session.sendRequest(request);
            std::istream& is = session.receiveResponse(_res);
            return RES { is, _res };
        }
        catch (Poco::Exception & exc) {
            OutputDebugStringA(exc.displayText().c_str());
        }
    });
}

main.cpp

Poco::Net::initializeSSL();

    Poco::URI uri("https://www.google.com");
    const Poco::Net::Context::Ptr context = new Poco::Net::Context(Poco::Net::Context::CLIENT_USE, "", "", "", Poco::Net::Context::VERIFY_NONE, 9, false, "ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH");
    Poco::Net::HTTPSClientSession session(uri.getHost(), uri.getPort(), context);

    std::future<RES> response = HttpClient::get(session, uri, {});
    response.get();

This is the precise error I got: C2280: response &response::operator =(const response &)': attempting to reference a deleted function future:line 332.

Thank you!

  • You are returning a reference to a local variable (`_res`) with `return RES { is, _res };`. Using it in any way afterward is undefined behavior. What exactly is your reason for `RES` containing references? – Max Langhof Oct 21 '19 at 16:43
  • How could I dereference is and _res before putting them into RES then? If I try *is or *_res it says no operator exists for these operands. I'm sorry if these questions seem a little basic, I'm pretty new to c++. – WubbaLubbaDubbDubb Oct 21 '19 at 16:47
  • The names are a bit confusing, but you can only dereference pointers. References are "just another name for an object", you use them like the object itself. In practical terms, you might want to make `RES` contain a pointer to the `std::istream` (the pointer can be copied). Try storing the `HTTPResponse` by value (no reference or pointer), but I can't tell immediately from the documentation whether it can be copy-constructed. I will note that diving into `async` (and the associated object lifetime challenges) without a good grasp of pointers and references seems ambitious... – Max Langhof Oct 21 '19 at 16:54
  • Class `HttpResponse` is not copyable, so it is allowed only to create one instance of that class and just refer to it. `RES` could keep smart pointer to `HttpResponse` for example `std::shared_ptr`. Also to avoid using `pointer/reference` to `istream` you can just extract all content of body out of this stream before returning from async and store it in `string`. `struct RES { std::string body; std::shared_ptr header; }`. – rafix07 Oct 21 '19 at 18:19
  • You use `async` to make task in background which will process the response, so why defer parsing stream in main thread? Just do it in this initiated async task. – rafix07 Oct 21 '19 at 18:24

2 Answers2

1

The error tells you that response objects can't be copied and that you are trying to do just that.

struct response { 
    std::istream& is; // can't be copied: istream( const istream&) = delete;
    Poco::Net::HTTPResponse& response; // also not copyable or movable
};

There is however nothing in the code you've shown that tries to do this.

receiveResponse() returns a reference to a std::istream which becomes problematic in case it throws. When you catch an exception you don't have anything to return, so you don't - and enter the land of Undefined Behaviour.

You might as well read the data inside the async lambda and store that in your RES directly.

#include <Poco/Exception.h>
#include <Poco/Net/HTTPRequest.h>
#include <Poco/Net/HTTPResponse.h>
#include <Poco/Net/HTTPSClientSession.h>
#include <Poco/Net/SecureStreamSocket.h>
#include <Poco/URI.h>

#include <future>
#include <iostream>
#include <map>
#include <vector>
#include <memory>

// a slightly modified version of your RES struct
struct RES {
    std::vector<std::string> data{}; // the document data

    // using a unique_ptr to make HTTPResponse easier to move
    std::unique_ptr<Poco::Net::HTTPResponse> 
        response = std::make_unique<Poco::Net::HTTPResponse>();

    bool ok = false;                 // if reading was done without an exception
};

class HttpClient {
public:
    static std::future<RES> get(Poco::Net::HTTPSClientSession& session,
                                Poco::URI url,
                                std::map<std::string, std::string> headers = {});
};

std::future<RES> HttpClient::get(Poco::Net::HTTPSClientSession& session,
                                 Poco::URI url,
                                 std::map<std::string, std::string> headers) {
    return std::async(std::launch::async, [&session, url, headers]() {

        RES res;

        try {
            Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET,
                                           url.getPathAndQuery(),
                                           Poco::Net::HTTPMessage::HTTP_1_1);

            // add request headers
            for(const auto&[field, value]:  headers)
                request.add(field, value);

            session.sendRequest(request);
            std::istream& is = session.receiveResponse(*res.response);

            // read document data
            std::string line;
            while(std::getline(is, line))
                res.data.push_back(line);

            res.ok = true; // reading was done without an exception
        } catch(Poco::Exception& exc) {
            std::cout << exc.displayText().c_str() << "\n";
        }

        // always return according to what you declared
        return res;
    });
}

Example usage:

class SSLInitializer {
public:
    SSLInitializer() { Poco::Net::initializeSSL(); }
    ~SSLInitializer() { Poco::Net::uninitializeSSL(); }
};

int main() {
    SSLInitializer sslInitializer;

    Poco::URI uri{"https://www.google.com/"};

    const Poco::Net::Context::Ptr context = new Poco::Net::Context(
        Poco::Net::Context::CLIENT_USE, "", "", "", Poco::Net::Context::VERIFY_NONE, 9,
        false, "ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH");

    Poco::Net::HTTPSClientSession sess(uri.getHost(), uri.getPort(), context);

    std::future<RES> fut_res = HttpClient::get(sess, uri);

    fut_res.wait();

    RES res = fut_res.get();
    std::cout << std::boolalpha << "Response OK: " << res.ok << "\n---\n";
    if(res.ok) {
        Poco::Net::HTTPResponse& header = *res.response

        std::cout << "HTTPResponse header:\n";
        for(const auto& [field, value] : header) {
            std::cout << field << " = " << value << "\n";
        }

        std::cout << "--- DOCUMENT DATA ---\n";
        for(const auto& s : res.data) {
            std::cout << s << "\n";
        }
    }
}
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
  • 2
    Note that `is` is a reference. References are never copy-assignable. – NathanOliver Oct 21 '19 at 16:33
  • @NathanOliver Yeah, that too :-) – Ted Lyngmo Oct 21 '19 at 16:33
  • So what could I do to fix this? – WubbaLubbaDubbDubb Oct 21 '19 at 16:34
  • https://stackoverflow.com/questions/31264984/c-compiler-error-c2280-attempting-to-reference-a-deleted-function-in-visual – dorKKnight Oct 21 '19 at 16:38
  • @WubbaLubbaDubbDubb You must try to figure out the exact line where you get the error and work from there. – Ted Lyngmo Oct 21 '19 at 16:47
  • @dorKKnight Sorry, I'm pretty new to c++ and I can't make heads or tails of that question. – WubbaLubbaDubbDubb Oct 21 '19 at 16:48
  • @WubbaLubbaDubbDubb It wasn't a question :) Doesn't the compiler tell you exactly where you are attempting to reference a deleted function? – Ted Lyngmo Oct 21 '19 at 16:49
  • @TedLyngmo Yeah I mean the StackOverflow question he referenced. Strangely enough, there's actually no line number or file name referenced in the error. It's just future:332. Maybe it's because I'm using the Visual Studio compiler. – WubbaLubbaDubbDubb Oct 21 '19 at 16:53
  • @WubbaLubbaDubbDubb the SO link mentioned earlier has the same error (in a different context). Please, go through the answers given in that SO thread, especially the one given by Barry. Barry's answer explains the reason behind this error besides suggesting a fix. – dorKKnight Oct 21 '19 at 17:00
  • @WubbaLubbaDubbDubb I made a version returning both document data and the `HTTPResponse` in the `std::future`. It should be pretty close to what you tried to accomplish. – Ted Lyngmo Oct 22 '19 at 03:41
  • 1
    @WubbaLubbaDubbDubb Glad to hear it! You are welcome! – Ted Lyngmo Oct 22 '19 at 13:06
0

The problem you're having is due to the class Poco::Net::HTTPResponse being non-copyable. It's copy constructor and assignment operator are both declared as private. So it is not possible to make a copy a copy of it.

I really do think it is overkill to spawn a new thread for each http request. I can understand your reasoning for wanting to do it, but you must bear in mind that there is some overhead involved in creating a new thread. You are better off just using the Poco classes or, if you want, a thing wrapper over them. You http requests are likely to run slower by spawning a new thread for each request.

May I suggest a slight change to your struct RES:

typedef struct response {
    Poco::Net::HTTPResponse::HTTPStatus status;
    std::string contents;
} RES;

This struct can now be used to hold data from the Poco::Net::HTTPResponse object after making a request. Then with the help of a helper function that outputs the contents of a std::istream into std::string:

std::string Gulp(std::istream& in)
{
    std::string response(std::istreambuf_iterator<char>(in), {});
    return response;
}

you can do this in main.cpp:

Poco::Net::initializeSSL();
Poco::URI uri("https://www.google.com");
const Poco::Net::Context::Ptr context = new 
Poco::Net::Context(Poco::Net::Context::CLIENT_USE, "", "", "", Poco::Net::Context::VERIFY_NONE, 9, false, "ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH");
Poco::Net::HTTPSClientSession session(uri.getHost(), uri.getPort(), context);

std::string path(uri.getPathAndQuery());
if (path.empty()) path = "/";

Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, path, Poco::Net::HTTPMessage::HTTP_1_1);
Poco::Net::HTTPResponse _res;
session.sendRequest(request);
std::istream& is = session.receiveResponse(_res);
RES response{ _res.getStatus(), Gulp(is) };
jignatius
  • 6,304
  • 2
  • 15
  • 30
  • So the reason that I'm making these async is because I'm going to be running sometimes dozens to hundreds of requests at a time using this app, so to make it all synchronized would make this app unusable. – WubbaLubbaDubbDubb Oct 22 '19 at 02:10
  • @WubbaLubbaDubbDubb What you could do is create a fixed number of threads at the beginning. Then use a threadsafe queue on which different jobs/requests are placed. A thread can pick a job off the queue when it is free. – jignatius Oct 22 '19 at 02:23