Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Roman Khabibov <roman.habibov@tarantool.org>,
	Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>,
	tarantool-patches@freelists.org
Subject: Re: [PATCH 0/2 v2] fio: show function name in all fio errors
Date: Fri, 7 Dec 2018 10:25:35 +0300	[thread overview]
Message-ID: <20181207072535.72jfepnrjj44vfnw@tkn_work_nb> (raw)
In-Reply-To: <20181204161239.3ngth5tqnt33ixk2@esperanza>

Hi all!

I don't think we should hide a real function where an error occurs. We
never know on what level the error was introduced. Let's see example:

```
local fio = require('fio')

local function read_app_config(app_name)
    local config_dir = os.getenv('TARANTOOL_APP_CONFIG_DIR') or '/etc'
    local config_path = fio.pathjoin(config_dir, app_name .. '.cfg.lua')
    dofile(config_path) -- exposes config = { ... } variable
    return config or {}
end

local function init()
    local config = read_app_config(os.getenv('TARANTOOL_APP_NAME'))
    ... do some initialization ...
end
```

When TARANTOOL_APP_NAME is not set in the example above the real error
is in the init() function and not in the read_app_config().

An application can use xpcall + debug.traceback to show or log
unexpected (or even expected) errors with full traceback.

I think within scope of this issue we should only add function names
into error messages and add path part number, but remove the second
parameter of the error() function where it is used across the fio
module.

WBR, Alexander Turenko.

On Tue, Dec 04, 2018 at 07:12:39PM +0300, Vladimir Davydov wrote:
> Alexander, please take a look.
> 
> On Tue, Dec 04, 2018 at 07:00:36PM +0300, Roman Khabibov wrote:
> > Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3580-err-msg-pathjoin
> > Issue: https://github.com/tarantool/tarantool/issues/3580
> > 
> > Roman Khabibov (1):
> >   lua: modify the error message in 'fio.pathjoin'
> > 
> > Vladislav Shpilevoy (1):
> >   fio: show function name in all fio errors
> > 
> >  src/lua/fio.lua       | 30 +++++++++++++++---------------
> >  test/app/fio.result   | 13 +++++++++++++
> >  test/app/fio.test.lua |  5 +++++
> >  3 files changed, 33 insertions(+), 15 deletions(-)

  reply	other threads:[~2018-12-07  7:25 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-04 16:00 Roman Khabibov
2018-12-04 16:00 ` [PATCH 1/2 v2] lua: modify the error message in 'fio.pathjoin' Roman Khabibov
2018-12-04 16:00 ` [PATCH 2/2 v2] fio: show function name in all fio errors Roman Khabibov
2018-12-04 16:12 ` [PATCH 0/2 " Vladimir Davydov
2018-12-07  7:25   ` Alexander Turenko [this message]
2018-12-08 13:41     ` [tarantool-patches] " roman
2018-12-08 14:36       ` Alexander Turenko
2018-12-08 15:02         ` roman
2018-12-08 15:12           ` Alexander Turenko
2018-12-09  9:36       ` Vladimir Davydov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181207072535.72jfepnrjj44vfnw@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=roman.habibov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH 0/2 v2] fio: show function name in all fio errors' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox