[tarantool-patches] [PATCH v1 1/1] box: updated httpc error message

[tarantool-patches] [PATCH v1 1/1] box: updated httpc error message

[Kirill Shcherbatov]

Previously Tarantool used to raise the confusing error message in
case of invalid usage of the httpc module. Fixed to follow the
current module API.

Closes #4136
---
http://github.com/tarantool/tarantool/tree/kshch/gh-4136-httpc-invalid-usage
https://github.com/tarantool/tarantool/issues/4136

 src/lua/httpc.lua                 | 2 +-
 test/app-tap/http_client.test.lua | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/lua/httpc.lua b/src/lua/httpc.lua
index cd44b6054..b767d14eb 100644
--- a/src/lua/httpc.lua
+++ b/src/lua/httpc.lua
@@ -294,7 +294,7 @@ curl_mt = {
         --
         request = function(self, method, url, body, opts)
             if not method or not url then
-                error('request(method, url [, options]])')
+                error('request(method, url [, body, [options]])')
             end
             local resp = self.curl:request(method, url, body, opts or {})
             if resp and resp.headers then
diff --git a/test/app-tap/http_client.test.lua b/test/app-tap/http_client.test.lua
index 0a323be9b..00dd3a251 100755
--- a/test/app-tap/http_client.test.lua
+++ b/test/app-tap/http_client.test.lua
@@ -62,10 +62,14 @@ local function stop_server(test, server)
 end
 
 local function test_http_client(test, url, opts)
-    test:plan(10)
+    test:plan(11)
 
     test:isnil(rawget(_G, 'http'), "global namespace is not polluted");
     test:isnil(rawget(_G, 'http.client'), "global namespace is not polluted");
+    local ok, err = pcall(client.request, client)
+    local usage_err = "request(method, url [, body, [options]])"
+    test:is_deeply({ok, tostring(err):split(': ')[2]}, {false, usage_err},
+                   "test httpc usage")
     local r = client.get(url, opts)
     test:is(r.status, 200, 'simple 200')
     test:is(r.reason, 'Ok', '200 - Ok')
-- 
2.21.0

[tarantool-patches] Re: [PATCH v1 1/1] box: updated httpc error message

[Alexander Turenko]

> box: updated httpc error message

We usually use prefix 'httpc:' for the http client.

On Mon, Apr 22, 2019 at 04:09:20PM +0300, Kirill Shcherbatov wrote:
> Previously Tarantool used to raise the confusing error message in
> case of invalid usage of the httpc module. Fixed to follow the
> current module API.
> 
> Closes #4136
> ---
> http://github.com/tarantool/tarantool/tree/kshch/gh-4136-httpc-invalid-usage
> https://github.com/tarantool/tarantool/issues/4136
> 
>  src/lua/httpc.lua                 | 2 +-
>  test/app-tap/http_client.test.lua | 6 +++++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lua/httpc.lua b/src/lua/httpc.lua
> index cd44b6054..b767d14eb 100644
> --- a/src/lua/httpc.lua
> +++ b/src/lua/httpc.lua
> @@ -294,7 +294,7 @@ curl_mt = {
>          --
>          request = function(self, method, url, body, opts)
>              if not method or not url then
> -                error('request(method, url [, options]])')
> +                error('request(method, url [, body, [options]])')

`request(method, url[, body[, options]])` seems to be more consistent
for me.

>              end
>              local resp = self.curl:request(method, url, body, opts or {})
>              if resp and resp.headers then
> diff --git a/test/app-tap/http_client.test.lua b/test/app-tap/http_client.test.lua
> index 0a323be9b..00dd3a251 100755
> --- a/test/app-tap/http_client.test.lua
> +++ b/test/app-tap/http_client.test.lua
> @@ -62,10 +62,14 @@ local function stop_server(test, server)
>  end
>  
>  local function test_http_client(test, url, opts)
> -    test:plan(10)
> +    test:plan(11)
>  
>      test:isnil(rawget(_G, 'http'), "global namespace is not polluted");
>      test:isnil(rawget(_G, 'http.client'), "global namespace is not polluted");
> +    local ok, err = pcall(client.request, client)
> +    local usage_err = "request(method, url [, body, [options]])"
> +    test:is_deeply({ok, tostring(err):split(': ')[2]}, {false, usage_err},
> +                   "test httpc usage")

I woudl replace 'test httpc usage' with 'test httpc usage error'.

tostring() is not necessary here (the error is just a Lua string).

Please add 'gh-xxxx: short description' comment.

>      local r = client.get(url, opts)
>      test:is(r.status, 200, 'simple 200')
>      test:is(r.reason, 'Ok', '200 - Ok')
> -- 
> 2.21.0
> 

[tarantool-patches] Re: [PATCH v1 1/1] box: updated httpc error message

[Kirill Shcherbatov]

> I woudl replace 'test httpc usage' with 'test httpc usage error'.
> 
> tostring() is not necessary here (the error is just a Lua string).
> 
> Please add 'gh-xxxx: short description' comment.
Done. Ok.

Previously Tarantool used to raise the confusing error message in
case of invalid usage of the httpc module. Fixed to follow the
current module API.

Closes #4136
---
 src/lua/httpc.lua                 | 2 +-
 test/app-tap/http_client.test.lua | 8 +++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/lua/httpc.lua b/src/lua/httpc.lua
index cd44b6054..b767d14eb 100644
--- a/src/lua/httpc.lua
+++ b/src/lua/httpc.lua
@@ -294,7 +294,7 @@ curl_mt = {
         --
         request = function(self, method, url, body, opts)
             if not method or not url then
-                error('request(method, url [, options]])')
+                error('request(method, url [, body, [options]])')
             end
             local resp = self.curl:request(method, url, body, opts or {})
             if resp and resp.headers then
diff --git a/test/app-tap/http_client.test.lua b/test/app-tap/http_client.test.lua
index 0a323be9b..01f6de5c0 100755
--- a/test/app-tap/http_client.test.lua
+++ b/test/app-tap/http_client.test.lua
@@ -62,7 +62,13 @@ local function stop_server(test, server)
 end
 
 local function test_http_client(test, url, opts)
-    test:plan(10)
+    test:plan(11)
+
+    -- gh-4136: confusing httpc usage error message
+    local ok, err = pcall(client.request, client)
+    local usage_err = "request(method, url [, body, [options]])"
+    test:is_deeply({ok, err:split(': ')[2]}, {false, usage_err},
+                   "test httpc usage error")
 
     test:isnil(rawget(_G, 'http'), "global namespace is not polluted");
     test:isnil(rawget(_G, 'http.client'), "global namespace is not polluted");
-- 
2.21.0

[tarantool-patches] Re: [PATCH v1 1/1] box: updated httpc error message

[Kirill Yukhin]

Hello,

On 22 Apr 16:09, Kirill Shcherbatov wrote:
> Previously Tarantool used to raise the confusing error message in
> case of invalid usage of the httpc module. Fixed to follow the
> current module API.
> 
> Closes #4136
> ---
> http://github.com/tarantool/tarantool/tree/kshch/gh-4136-httpc-invalid-usage
> https://github.com/tarantool/tarantool/issues/4136

I've checked your patch into master and 2.1 branch.

--
Regards, Kirill Yukhin