[Tarantool-patches] [PATCH] error: add format string usage to form a CustomError message

[Tarantool-patches] [PATCH] error: add format string usage to form a CustomError message

[Leonid Vasiliev]

For the CustomError the ability to create a message
using a format string was added.

Closes #4903

@TarantoolBot document
Title: Add format string usage to form a CustomError message
When creating a ClientError error the predefined format
(corresponding with the error code) is used.
When creating a CustomError error a format string can be
used to form the message.

ClientError:
```Lua
box.error(code, reason args)
```

CustomError:
```Lua
box.error(type, reason format string, reason args)
```
---
https://github.com/tarantool/tarantool/issues/4903
https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4903-format-string-for-CustomError
@ChangeLog Add format string usage to form a CustomError message

 src/box/lua/error.cc    | 33 +++++++++++++++++++++++----------
 test/box/error.result   | 10 ++++++++++
 test/box/error.test.lua |  5 +++++
 3 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/src/box/lua/error.cc b/src/box/lua/error.cc
index e0a3e73..54ec284 100644
--- a/src/box/lua/error.cc
+++ b/src/box/lua/error.cc
@@ -43,15 +43,18 @@ extern "C" {
 #include "box/error.h"
 
 /**
- * Parse Lua arguments (they can come as single table
- * f({code : number, reason : string}) or as separate members
- * f(code, reason)) and construct struct error with given values.
+ * Parse Lua arguments (they can come as single table or as
+ * separate members) and construct struct error with given values.
  *
- * Instead of 'code' it is possible to specify a string name of
- * the error object's type:
+ * Can be used either the 'code' (numeric) for create a ClientError
+ * error with corresponding message (the format is predefined)
+ * and type or the 'type' (string) for create a CustomError error
+ * with custom type and desired message.
  *
- *     box.error(type, reason, ...)
- *     box.error({type = string, reason = string, ...})
+ *     box.error(code, reason args)
+ *     box.error({code = num, reason = string, ...})
+ *     box.error(type, reason format string, reason args)
+ *     box.error({type = string, code = num, reason = string, ...})
  *
  * In case one of arguments is missing its corresponding field
  * in struct error is filled with default value.
@@ -69,12 +72,21 @@ luaT_error_create(lua_State *L, int top_base)
 	int top_type = lua_type(L, top_base);
 	if (top >= top_base && (top_type == LUA_TNUMBER ||
 				top_type == LUA_TSTRING)) {
+		/* Shift of the "reason args". */
+		int shift = 1;
 		if (top_type == LUA_TNUMBER) {
 			code = lua_tonumber(L, top_base);
 			reason = tnt_errcode_desc(code);
 		} else {
 			custom_type = lua_tostring(L, top_base);
-			reason = "%s";
+			/*
+			 * For the CustomError, the message format
+			 * must be set via a function argument.
+			 */
+			if (lua_type(L, top_base + 1) != LUA_TSTRING)
+				return NULL;
+			reason = lua_tostring(L, top_base + 1);
+			shift = 2;
 		}
 		if (top > top_base) {
 			/* Call string.format(reason, ...) to format message */
@@ -85,9 +97,10 @@ luaT_error_create(lua_State *L, int top_base)
 			if (lua_isnil(L, -1))
 				goto raise;
 			lua_pushstring(L, reason);
-			for (int i = top_base + 1; i <= top; i++)
+			int nargs = 1;
+			for (int i = top_base + shift; i <= top; ++i, ++nargs)
 				lua_pushvalue(L, i);
-			lua_call(L, top - top_base + 1, 1);
+			lua_call(L, nargs, 1);
 			reason = lua_tostring(L, -1);
 		} else if (strchr(reason, '%') != NULL) {
 			/* Missing arguments to format string */
diff --git a/test/box/error.result b/test/box/error.result
index 59a5301..2196fa5 100644
--- a/test/box/error.result
+++ b/test/box/error.result
@@ -956,3 +956,13 @@ s:drop()
 box.schema.func.drop('runtimeerror')
  | ---
  | ...
+
+-- gh-4903: add format string usage for a CustomError message
+--
+err = box.error.new('TestType', 'Message arg1: %s. Message arg2: %u', '1', 2)
+ | ---
+ | ...
+err.message
+ | ---
+ | - 'Message arg1: 1. Message arg2: 2'
+ | ...
diff --git a/test/box/error.test.lua b/test/box/error.test.lua
index d63e835..ed95d1d 100644
--- a/test/box/error.test.lua
+++ b/test/box/error.test.lua
@@ -270,3 +270,8 @@ gc_err
 
 s:drop()
 box.schema.func.drop('runtimeerror')
+
+-- gh-4903: add format string usage for a CustomError message
+--
+err = box.error.new('TestType', 'Message arg1: %s. Message arg2: %u', '1', 2)
+err.message
-- 
2.7.4

Re: [Tarantool-patches] [PATCH] error: add format string usage to form a CustomError message

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

On 28/05/2020 10:31, Leonid Vasiliev wrote:
> For the CustomError the ability to create a message
> using a format string was added.
> 
> Closes #4903
> 
> @TarantoolBot document
> Title: Add format string usage to form a CustomError message
> When creating a ClientError error the predefined format
> (corresponding with the error code) is used.
> When creating a CustomError error a format string can be
> used to form the message.
> 
> ClientError:
> ```Lua
> box.error(code, reason args)
> ```
> 
> CustomError:
> ```Lua
> box.error(type, reason format string, reason args)
> ```

It would be good to provide an example here. And to state, that
client error case is not changed. Otherwise from the text above
it looks like you changed it to be 'the predefined format is used'.
But I will leave it to the doc team to ping you after this is
pushed.

Talking of the client error formatting correctness, it looks like
currently the documentation says:
https://www.tarantool.io/en/doc/2.3/reference/reference_lua/box_error/#lua-function.box.error

	When called with a Lua-table argument, the code and reason have any user-desired values. The result will be those values.

	Parameters:

	reason (string) – description of an error, defined by user
	code (integer) – numeric code for this error, defined by user

So in the site it is said, that 'reason' is defined by user, not be
a predefined internal format. The predefined format is used in
box.error(code, ...) syntax.

> ---
> https://github.com/tarantool/tarantool/issues/4903
> https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4903-format-string-for-CustomError
> @ChangeLog Add format string usage to form a CustomError message

Re: [Tarantool-patches] [PATCH] error: add format string usage to form a CustomError message

[Leonid Vasiliev]

Hi! Thank you for the review.
I updated the commit message:

error: add format string usage to compose a CustomError message

For the CustomError the ability to create a message
using a format string was added.

Closes #4903

@TarantoolBot document
Title: Add format string usage to compose a CustomError message
When an error created using the separate members mode
(box.error(code, errtext[, errtext ...])) in the case of
ClientError error creation, a pre-defined format is used
(corresponding to the error code) (nothing has changed), in
the case of CustomError error creation, a format string can
be used to compose a message.

ClientError(nothing has changed):
```Lua
box.error(code, reason args)
```

Example for ER_CREATE_SPACE:
```Lua
box.error(9, "my_space", "reason")
```
Result:
```Lua
error: 'Failed to create space ''my_space'': reason'
```

CustomError:
```Lua
box.error(type, reason format string, reason args)
```
Example:
```Lua
box.error("MyCustomType", "The error reason: %s", "some error reason")
```
Result:
```Lua
error: 'The error reason: some error reason'
```

On 30.05.2020 01:05, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> On 28/05/2020 10:31, Leonid Vasiliev wrote:
>> For the CustomError the ability to create a message
>> using a format string was added.
>>
>> Closes #4903
>>
>> @TarantoolBot document
>> Title: Add format string usage to form a CustomError message
>> When creating a ClientError error the predefined format
>> (corresponding with the error code) is used.
>> When creating a CustomError error a format string can be
>> used to form the message.
>>
>> ClientError:
>> ```Lua
>> box.error(code, reason args)
>> ```
>>
>> CustomError:
>> ```Lua
>> box.error(type, reason format string, reason args)
>> ```
> 
> It would be good to provide an example here. And to state, that
> client error case is not changed. Otherwise from the text above
> it looks like you changed it to be 'the predefined format is used'.
> But I will leave it to the doc team to ping you after this is
> pushed.
> 
> Talking of the client error formatting correctness, it looks like
> currently the documentation says:
> https://www.tarantool.io/en/doc/2.3/reference/reference_lua/box_error/#lua-function.box.error
> 
> 	When called with a Lua-table argument, the code and reason have any user-desired values. The result will be those values.
> 
> 	Parameters:
> 
> 	reason (string) – description of an error, defined by user
> 	code (integer) – numeric code for this error, defined by user
> 
> So in the site it is said, that 'reason' is defined by user, not be
> a predefined internal format. The predefined format is used in
> box.error(code, ...) syntax.
> 
>> ---
>> https://github.com/tarantool/tarantool/issues/4903
>> https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4903-format-string-for-CustomError
>> @ChangeLog Add format string usage to form a CustomError message

Re: [Tarantool-patches] [PATCH] error: add format string usage to form a CustomError message

[Vladislav Shpilevoy]

LGTM.

Re: [Tarantool-patches] [PATCH] error: add format string usage to form a CustomError message

[Kirill Yukhin]

Hello,

On 28 май 11:31, Leonid Vasiliev wrote:
> For the CustomError the ability to create a message
> using a format string was added.
> 
> Closes #4903

LGTM.

I've checked your patch into 2.4 and master.

--
Regards, Kirill Yukhin