<div id="geary-body"><div><span style="white-space: pre-wrap;">Hi all! Alexander, thanks for review. Vlad, can you do a second review, please?

On 05.02.2019 7:40, Alexander Turenko wrote:
</span><div class="geary-quote-container geary-controllable" style="white-space: pre-wrap;"><div class="geary-quote"><blockquote>The commit message header:

<blockquote>httpc: set reason when status is more than 400
</blockquote>More or equal.

<blockquote>Set the reason "Unknown" when it is CURLE_OK and status is more than 400.
</blockquote></blockquote></div><div class="geary-hider"><button class="geary-button" type="button">▲        ▲        ▲</button></div></div><span style="white-space: pre-wrap;">Done.
</span><div class="geary-quote-container geary-controllable" style="white-space: pre-wrap;"><div class="geary-quote"><blockquote>The >= 100 condition is matter too.
<a href="https://curl.haxx.se/libcurl/c/CURLINFO_RESPONSE_CODE.html">https://curl.haxx.se/libcurl/c/CURLINFO_RESPONSE_CODE.html</a> states you
can get zero as a code if 'no server response code has been received'.
Don't sure how to reproduce this case, but anyway I think the commit
message (and the header) should be either describe the whole change or
be a bit more common. Say:

```
httpc: set 'Unknown' reason for 0, 4xx, 5xx codes
```
</blockquote></div><div class="geary-hider"><button class="geary-button" type="button">▲        ▲        ▲</button></div></div><span style="white-space: pre-wrap;">
Done.


</span><div class="geary-quote-container" style="white-space: pre-wrap;"><div class="geary-quote"><blockquote>Why this order? req->status >= 100 && req->status < 400 reads more
easily.
</blockquote></div></div><span style="white-space: pre-wrap;">
Done.</span></div><div><span style="white-space: pre-wrap;"><br></span></div><div><span style="white-space: pre-wrap;">commit a39ede25caef257fdc99b7896abb31557773e7f5
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date:   Mon Feb 4 16:28:30 2019 +0300

    httpc: set 'Unknown' reason for 0, 4xx, 5xx codes
    
    Set the reason "Unknown" when it is CURLE_OK and status is more than or equal
    to 400.
    
    Closes #3681

diff --git a/src/httpc.c b/src/httpc.c
index 950f8b32f..1f9366982 100644
--- a/src/httpc.c
+++ b/src/httpc.c
@@ -325,8 +325,12 @@ httpc_execute(struct httpc_request *req, double timeout)
        case CURLE_OK:
                curl_easy_getinfo(req->curl_request.easy, CURLINFO_RESPONSE_CODE, &longval);
                req->status = (int) longval;
-               /* TODO: get real response string from resp->headers */
-               req->reason = "Ok";
+
+               if (req->status >= 100 && req->status < 400)
+                       req->reason = "Ok";
+               else
+                       req->reason = "Unknown";
+
                if (req->status == 200) {
                        ++env->stat.http_200_responses;
                } else {
diff --git a/test/app-tap/http_client.test.lua b/test/app-tap/http_client.test.lua
index 10a3728f8..d4f3a9f45 100755
--- a/test/app-tap/http_client.test.lua
+++ b/test/app-tap/http_client.test.lua
@@ -62,12 +62,13 @@ local function stop_server(test, server)
 end
 
 local function test_http_client(test, url, opts)
-    test:plan(9)
+    test:plan(10)
 
     test:isnil(rawget(_G, 'http'), "global namespace is not polluted");
     test:isnil(rawget(_G, 'http.client'), "global namespace is not polluted");
     local r = client.get(url, opts)
     test:is(r.status, 200, 'simple 200')
+    test:is(r.reason, 'Ok', '200 - Ok')
     test:is(r.proto[1], 1, 'proto major http 1.1')
     test:is(r.proto[2], 1, 'proto major http 1.1')
     test:ok(r.body:match("hello") ~= nil, "body")
@@ -104,7 +105,7 @@ local function test_cancel_and_errinj(test, url, opts)
 end
 
 local function test_post_and_get(test, url, opts)
-    test:plan(19)
+    test:plan(21)
 
     local http = client.new()
     test:ok(http ~= nil, "client is created")
@@ -159,6 +160,7 @@ local function test_post_and_get(test, url, opts)
 
     r = responses.absent_get
     test:is(r.status, 500, "GET: absent method http code page exists")
+    test:is(r.reason, 'Unknown', '500 - Unknown')
     test:is(r.body, "No such method", "GET: absent method right body")
 
     r = responses.empty_post
@@ -180,6 +182,7 @@ local function test_post_and_get(test, url, opts)
 
     r = responses.bad_get
     test:is(r.status, 404, "GET: http page not exists")
+    test:is(r.reason, 'Unknown', '404 - Unknown')
     test:isnt(r.body:len(), 0, "GET: not empty body page not exists")
     test:ok(string.find(r.body, "Not Found"),
                 "GET: right body page not exists")
</span></div><div><span style="white-space: pre-wrap;"><br></span></div></div>