venediktov / vanilla-rtb

Real Time Bidding (RTB) - Demand Side Platform framework
http://forkbid.com
GNU General Public License v3.0
318 stars 85 forks source link

How to return HTTP Status Code 204? #50

Closed bkda closed 7 years ago

bkda commented 7 years ago

Hi, these days I get stuck with another problem, can I return custom http status code?

I read some parts of this project and CRUD project, and I find this,

        request_parser_.reset();
        reply_.headers.resize(0);
        reply_.status = reply::status_type::ok;
        reply_.content="";
        request_ = request(); 
        do_read();

https://github.com/venediktov/CRUD/blob/master/service/persistent_connection.hpp#L123

https://github.com/venediktov/CRUD/blob/master/service/mime_types.cpp#L42

Here's the situation, I want to return 204 http status code, but can't find a way to do this trick. Is this possible? Similarly, some platforms need the Content-Type to be set application/json, so maybe it's a improvement to make the http response header more customizable.

Many thanks.

venediktov commented 7 years ago

Hi, do you want to respond with 204 for no-bid ? Ideally we should provide an option to do no-bid with HTTP 204 response. Currently you have to manually hack it where you have access to http::server::reply structure do

if (wire_response && timer.expires_from_now().total_milliseconds() > 0) {
                    r << to_string(*wire_response) << http::server::reply::flush("");
                } else {
                    r << http::server::reply::flush("");
                }

so you can hack it by checking auction_response variable this is your BidResponse and if you have no bid then

r = http::server::reply::stock_reply(http::server::reply::no_content); //204

Let me know if I understand you correctly for 204 response . Do you think more custom responses needed ?

2.) As far as the mime types you can do this r << http::server::reply::flush("json"); That will set Content-Type to application/json But it will override status to OK 200

Let's keep this issue open and we will look into better solution .

bkda commented 7 years ago

r = http::server::reply::stock_reply(http::server::reply::no_content); //204

The above code works, and it can return 204 http status code now, but Content-Type returns text/plain which is need to be reset.

Thanks for your quick response.

bkda commented 7 years ago

@venediktov

Hi, I tried this way to set Content-Type to application/json, but it still doesn't work:

                if (res.seatbid.size() == 0){
                    r = http::server::reply::stock_reply(http::server::reply::no_content); //204
                }else if (wire_response && timer.expires_from_now().total_milliseconds() > 0) {
                    r << to_string(*wire_response) << http::server::reply::flush("json");
                } else {
                    r << http::server::reply::flush("json");
                }

Response :

HTTP/1.0 200 OK Content-Length: 2328 Content-Type: text/plain Connection:: keep-alive

How can I fix this? If I missed something, please let me know.

Another question is that if I don't need a regex to match the url, I just want to start a simple server with a url http://localhost:port, how can I remove the crud_match part and run this server?

Best.

venediktov commented 7 years ago

Hi, I am working on CRUD package and exchange_handler modification so that you could do

exchange_handler<DSLT> openrtb_handler(std::chrono::milliseconds(config.data().handler_timeout_v1));
    openrtb_handler
    .if_response([](const auto &bid_response) { //positive means response is valid
       return bid_response.seatbid.size();
    })
    .reply([](const auto reply &r ) {
        r << http::server::reply::flush_204("json"); //still undecided on the use case
    })
    .auction_async([](const auto &request) {
        return  BidResponse();
});

But I am trying to design it so it works with old code too. Hopefully today I should have my pull request ready .

2.) Your question with regards to regex ... you can use CRUD without regex https://github.com/venediktov/vanilla-rtb/blob/master/CRUD/main.cpp#L70

 simple_handler.crud_match(std::string("/venue_handler/RTB") )
        .post([](http::server::reply & r, const http::crud::crud_match<std::string> & match) {
            r << "OK" << http::server::reply::flush("text") ;
            std::cout << "POST request_data=" << match.data << std::endl;
});

Best, Vlad.

venediktov commented 7 years ago

Hi TrasyDa, I have created a pull request https://github.com/venediktov/vanilla-rtb/pull/51 and waiting for approval . The new proposed use case for NoBid please find under .if_response() handler. It works by returning a valid callback from this if_response() handler. if you detected some condition that require custom response then simply return a handler in our case I returned C++11 lambda return [](http::server::reply &r) { r = http::server::reply::stock_reply(http::server::reply::no_content);} ; The calll back returned from the if_response handler should be of the following type std::function<void (http::server::reply)> So if_response is just another handler that accepts BidRequest and returns custom response handler in case some conditions are met ( in your case bid_response.seatbid.size() == 0 )

2.) I have also changed CRUD and for 204 no Content-Type or Content-Length and no Body is sent back according to HTTP standards, because 204 is No Content.

exchange_handler<DSLT> openrtb_handler(std::chrono::milliseconds(config.data().handler_timeout_v1));
    openrtb_handler
    .logger([](const std::string &data) {
//        LOG(debug) << "request_data_v1=" << data ;
    })
    .error_logger([](const std::string &data) {
        LOG(debug) << "request v1 error " << data ;
    })
    .if_response([](const auto &bid_response) {
      if ( bid_response.seatbid.size() == 0 ) {//no bid HTTP 204
          return [](http::server::reply &r) { r = http::server::reply::stock_reply(http::server::reply::no_content);} ;
      }//else returns empty std::function<>
    })
    .auction_async([](const auto &request) {
        //This creates seatbid.size() == 0
        return  BidResponse();
    });
venediktov commented 7 years ago

Hi TracyDa, I am closing this issue please refer to pull request 50K QPS + Http 204 for NoBid #51 If you have any problems you are welcome to re-open this issue or create a new one. Thank you for your interest in vanilla-rtb

venediktov commented 7 years ago

There is a bug in initial implementation , it redundantly creates wire response from BidResponse but sends 204 as designed. Created issue #67