[PATCH v3] fio: cleanup error messages

[PATCH v3] fio: cleanup error messages

[Roman Khabibov]

Otherwise it is hard to debug code, throwing
exceptions. fio.pathjoin was just one example among
many.

Closes #3580

Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3580-err-msg-pathjoin
Issue: https://github.com/tarantool/tarantool/issues/3580
---
 src/lua/fio.lua       | 18 ++++++-------
 test/app/fio.result   | 61 +++++++++++++++++++++++++++++++++++++++++++
 test/app/fio.test.lua | 19 +++++++++++++-
 3 files changed, 88 insertions(+), 10 deletions(-)

diff --git a/src/lua/fio.lua b/src/lua/fio.lua
index 55faebdcb..f92267023 100644
--- a/src/lua/fio.lua
+++ b/src/lua/fio.lua
@@ -126,7 +126,7 @@ fio_methods.seek = function(self, offset, whence)
     end
     if type(whence) == 'string' then
         if fio.c.seek[whence] == nil then
-            error(sprintf("Unknown whence: %s", whence))
+            error(sprintf("Usage: fio.seek() unknown whence: %s", whence))
         end
         whence = fio.c.seek[whence]
     else
@@ -164,7 +164,7 @@ fio.open = function(path, flags, mode)
     local iflag = 0
     local imode = 0
     if type(path) ~= 'string' then
-        error("Usage open(path[, flags[, mode]])")
+        error("Usage: fio.open(path[, flags[, mode]])")
     end
     if type(flags) ~= 'table' then
         flags = { flags }
@@ -179,7 +179,7 @@ fio.open = function(path, flags, mode)
             iflag = bit.bor(iflag, flag)
         else
             if fio.c.flag[ flag ] == nil then
-                error(sprintf("Unknown flag: %s", flag))
+                error(sprintf("Usage: fio.open() unknown flag: %s", flag))
             end
             iflag = bit.bor(iflag, fio.c.flag[ flag ])
         end
@@ -188,7 +188,7 @@ fio.open = function(path, flags, mode)
     for _, m in pairs(mode) do
         if type(m) == 'string' then
             if fio.c.mode[m] == nil then
-                error(sprintf("Unknown mode: %s", m))
+                error(sprintf("Usage: fio.open() unknown mode: %s", m))
             end
             imode = bit.bor(imode, fio.c.mode[m])
         else
@@ -213,7 +213,7 @@ fio.pathjoin = function(...)
     while i <= len do
         local sp = select(i, ...)
         if sp == nil then
-            error("Undefined path part "..i)
+            error("Usage: fio.pathjoin() undefined path part: "..i)
         end
 
         sp = tostring(sp)
@@ -233,7 +233,7 @@ fio.pathjoin = function(...)
     while i <= len do
         local sp = select(i, ...)
         if sp == nil then
-            error("Undefined path part")
+            error("Usage: fio.pathjoin() undefined path part: "..i)
         end
 
         sp = tostring(sp)
@@ -254,7 +254,7 @@ end
 
 fio.basename = function(path, suffix)
     if type(path) ~= 'string' then
-        error("Usage fio.basename(path[, suffix])")
+        error("Usage: fio.basename(path[, suffix])")
     end
 
     path = tostring(path)
@@ -273,7 +273,7 @@ end
 
 fio.dirname = function(path)
     if type(path) ~= 'string' then
-        error("Usage fio.dirname(path)")
+        error("Usage: fio.dirname(path)")
     end
     path = ffi.new('char[?]', #path + 1, path)
     return ffi.string(ffi.C.dirname(path))
@@ -297,7 +297,7 @@ fio.abspath = function(path)
     -- following established conventions of fio module:
     -- letting nil through and converting path to string
     if path == nil then
-        error("Usage fio.abspath(path)")
+        error("Usage: fio.abspath(path)")
     end
     path = path
     local joined_path = ''
diff --git a/test/app/fio.result b/test/app/fio.result
index b7a1f65c6..d58683b69 100644
--- a/test/app/fio.result
+++ b/test/app/fio.result
@@ -1154,6 +1154,67 @@ ch:get() == hash
 ---
 - true
 ...
+-- gh-3580: Check that error messages are descriptive enough.
+fh1:seek(nil, 'a')
+---
+- error: 'builtin/fio.lua:129: Usage: fio.seek() unknown whence: a'
+...
+fio.open(nil)
+---
+- error: 'builtin/fio.lua:167: Usage: fio.open(path[, flags[, mode]])'
+...
+fio.open(tmp1, {'A'}, 0777)
+---
+- error: 'builtin/fio.lua:182: Usage: fio.open() unknown flag: A'
+...
+fio.open(tmp1, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, {'A'})
+---
+- error: 'builtin/fio.lua:191: Usage: fio.open() unknown mode: A'
+...
+fio.pathjoin(nil)
+---
+- error: 'builtin/fio.lua:216: Usage: fio.pathjoin() undefined path part: 1'
+...
+fio.pathjoin('abc', nil)
+---
+- error: 'builtin/fio.lua:236: Usage: fio.pathjoin() undefined path part: 2'
+...
+fio.pathjoin('abc', 'cde', nil)
+---
+- error: 'builtin/fio.lua:236: Usage: fio.pathjoin() undefined path part: 3'
+...
+fio.basename(nil)
+---
+- error: 'builtin/fio.lua:257: Usage: fio.basename(path[, suffix])'
+...
+fio.abspath(nil)
+---
+- error: 'builtin/fio.lua:300: Usage: fio.abspath(path)'
+...
+fio.chdir(1)
+---
+- error: 'builtin/fio.lua:322: Usage: fio.chdir(path)'
+...
+fio.listdir(1)
+---
+- error: 'builtin/fio.lua:329: Usage: fio.listdir(path)'
+...
+fio.mktree(1)
+---
+- error: 'builtin/fio.lua:348: Usage: fio.mktree(path[, mode])'
+...
+fio.rmtree(1)
+---
+- error: 'builtin/fio.lua:374: Usage: fio.rmtree(path)'
+...
+fio.copytree(nil, nil)
+---
+- error: 'builtin/fio.lua:420: Usage: fio.copytree(from, to)'
+...
+fio.copytree(nil, nil)
+---
+- error: 'builtin/fio.lua:420: Usage: fio.copytree(from, to)'
+...
 fio.unlink(tmp1)
 ---
 - true
diff --git a/test/app/fio.test.lua b/test/app/fio.test.lua
index 4f34fd11c..448f62a58 100644
--- a/test/app/fio.test.lua
+++ b/test/app/fio.test.lua
@@ -378,6 +378,23 @@ f1 = fiber.create(function() str = fh1:read() ch:put(digest.crc32(str)) end)
 f2 = fiber.create(function() str = fh2:read() end)
 ch:get() == hash
 
+-- gh-3580: Check that error messages are descriptive enough.
+fh1:seek(nil, 'a')
+fio.open(nil)
+fio.open(tmp1, {'A'}, 0777)
+fio.open(tmp1, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, {'A'})
+fio.pathjoin(nil)
+fio.pathjoin('abc', nil)
+fio.pathjoin('abc', 'cde', nil)
+fio.basename(nil)
+fio.abspath(nil)
+fio.chdir(1)
+fio.listdir(1)
+fio.mktree(1)
+fio.rmtree(1)
+fio.copytree(nil, nil)
+fio.copytree(nil, nil)
+
 fio.unlink(tmp1)
 fio.unlink(tmp2)
-fio.rmdir(tmpdir)
+fio.rmdir(tmpdir)
\ No newline at end of file
-- 
2.17.1

Re: [PATCH v3] fio: cleanup error messages

[Vladimir Davydov]

On Wed, Dec 12, 2018 at 01:46:14AM +0300, Roman Khabibov wrote:
> Otherwise it is hard to debug code, throwing
> exceptions. fio.pathjoin was just one example among
> many.
> 
> Closes #3580
> 
> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3580-err-msg-pathjoin
> Issue: https://github.com/tarantool/tarantool/issues/3580

Branch/Issue prefixes are not necessary - it's clear which is which
without them. Also, the links should go after the '---' below (this way
they are ignored by git-am). And you forgot a changelog - whenever you
resubmit a patch after making some changes you should write a brief
change log, e.g.

https://www.freelists.org/post/tarantool-patches/PATCH-v3-replication-fix-exit-with-ER-NO-SUCH-USER-during-bootstrap

> ---
>  src/lua/fio.lua       | 18 ++++++-------
>  test/app/fio.result   | 61 +++++++++++++++++++++++++++++++++++++++++++
>  test/app/fio.test.lua | 19 +++++++++++++-
>  3 files changed, 88 insertions(+), 10 deletions(-)
> 
> diff --git a/src/lua/fio.lua b/src/lua/fio.lua
> index 55faebdcb..f92267023 100644
> --- a/src/lua/fio.lua
> +++ b/src/lua/fio.lua
> @@ -126,7 +126,7 @@ fio_methods.seek = function(self, offset, whence)
>      end
>      if type(whence) == 'string' then
>          if fio.c.seek[whence] == nil then
> -            error(sprintf("Unknown whence: %s", whence))
> +            error(sprintf("Usage: fio.seek() unknown whence: %s", whence))

I meant that "Usage: ..." pattern should only be used for errors that
don't have any additional details. Sorry for misunderstanding. I amended
it by myself.

> +-- gh-3580: Check that error messages are descriptive enough.
> +fh1:seek(nil, 'a')
> +---
> +- error: 'builtin/fio.lua:129: Usage: fio.seek() unknown whence: a'
> +...

Forgot to tell you last time. These error messages would change if we
made any changes to fio.lua, which would be annoying. I filtered the
line numbers out (see test_run:cmd("push filter ...")).

>  fio.unlink(tmp1)
>  fio.unlink(tmp2)
> -fio.rmdir(tmpdir)
> +fio.rmdir(tmpdir)
> \ No newline at end of file

Newline was removed by your patch. Please fix your editor. I amended it
as well and pushed the patch to 2.1.