Re: [Tarantool-patches] [PATCH luajit v2 6/6] profilers: print user-friendly errors

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hello, Max


thanks for the patch. LGTM with minor comments


On 12/6/23 21:55, Maxim Kokryashkin wrote:


<snipped>

> diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua
> index 64c4a455..749f70db 100755
> --- a/tools/sysprof/parse.lua
> +++ b/tools/sysprof/parse.lua
> @@ -237,7 +237,7 @@ function M.parse(reader, symbols)
>     local _ = reader:read_octets(3)
>   
>     if magic ~= LJP_MAGIC then
> -    error("Bad LJP format prologue: "..magic)
> +    error("Bad LJP format prologue: " .. tostring(magic))
>     end

LJP is not a well-known abbreviation. Probably, it is better to write 
the error

message without it. I see that LJP/LJS is used extensively in error 
message, so feel free to

ignore or fix in a separate patch.

>   
>     if string.byte(version) ~= LJP_CURRENT_VERSION then
> diff --git a/tools/utils/safe_event_reader.lua b/tools/utils/safe_event_reader.lua
> new file mode 100644
> index 00000000..39246a9d
> --- /dev/null
> +++ b/tools/utils/safe_event_reader.lua
> @@ -0,0 +1,34 @@
> +local bufread = require('utils.bufread')
> +local symtab = require('utils.symtab')
> +
> +local function make_error_handler(fmt, inputfile)
> +  return function(err)
> +    io.stderr:write(string.format(fmt, inputfile))
> +    io.stderr:write(string.format('\nOriginal error: %s', err))
> +    os.exit(1, true)
> +  end
> +end
> +
> +local function safe_event_reader(parser, inputfile)
> +  local _, reader = xpcall(
> +    bufread.new,
> +    make_error_handler('Failed to open %s.', inputfile),
> +    inputfile
> +  )
> +
> +  local _, symbols = xpcall(
> +    symtab.parse,
> +    make_error_handler('Failed to parse symtab from %s.', inputfile),
> +    reader
> +  )
> +
> +  local _, events = xpcall(
> +    parser,
> +    make_error_handler('Failed to parse profile data from %s.', inputfile),
> +    reader,
> +    symbols
> +  )
> +  return events, symbols
> +end
> +
> +return safe_event_reader
> diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua
> index 0c878e7a..c4aefef7 100644
> --- a/tools/utils/symtab.lua
> +++ b/tools/utils/symtab.lua
> @@ -95,7 +95,7 @@ function M.parse(reader)
>     local _ = reader:read_octets(3)
>   
>     if magic ~= LJS_MAGIC then
> -    error("Bad LJS format prologue: "..magic)
> +    error("Bad LJS format prologue: " .. tostring(magic))

LJS is not a well-known abbreviation. Probably, it is better to write 
the error

message without it.

>     end
>   
>     if string.byte(version) ~= LJS_CURRENT_VERSION then