Consistent erros returned from EVAL scripts (#10218)

This PR handles inconsistencies in errors returned from lua scripts.
Details of the problem can be found in #10165.

### Changes

- Remove double stack trace. It's enough that a stack trace is automatically added by the engine's error handler
  see d0bc4fff18/src/function_lua.c (L472-L485)
  and d0bc4fff18/src/eval.c (L243-L255)
- Make sure all errors a preceded with an error code. Passing a simple string to `luaPushError()` will prepend it
  with a generic `ERR` error code.
- Make sure lua error table doesn't include a RESP `-` error status. Lua stores redis error's as a lua table with a
  single `err` field and a string. When the string is translated back to RESP we add a `-` to it.
  See d0bc4fff18/src/script_lua.c (L510-L517)
  So there's no need to store it in the lua table.

### Before & After
```diff
--- <unnamed>
+++ <unnamed>
@@ -1,14 +1,14 @@
  1: config set maxmemory 1
  2: +OK
  3: eval "return redis.call('set','x','y')" 0
- 4: -ERR Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479): @user_script:1: @user_script: 1: -OOM command not allowed when used memory > 'maxmemory'.
+ 4: -ERR Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479): @user_script:1: OOM command not allowed when used memory > 'maxmemory'.
  5: eval "return redis.pcall('set','x','y')" 0
- 6: -@user_script: 1: -OOM command not allowed when used memory > 'maxmemory'.
+ 6: -OOM command not allowed when used memory > 'maxmemory'.
  7: eval "return redis.call('select',99)" 0
  8: -ERR Error running script (call to 4ad5abfc50bbccb484223905f9a16f09cd043ba8): @user_script:1: ERR DB index is out of range
  9: eval "return redis.pcall('select',99)" 0
 10: -ERR DB index is out of range
 11: eval_ro "return redis.call('set','x','y')" 0
-12: -ERR Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479): @user_script:1: @user_script: 1: Write commands are not allowed from read-only scripts.
+12: -ERR Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479): @user_script:1: ERR Write commands are not allowed from read-only scripts.
 13: eval_ro "return redis.pcall('set','x','y')" 0
-14: -@user_script: 1: Write commands are not allowed from read-only scripts.
+14: -ERR Write commands are not allowed from read-only scripts.
```
This commit is contained in:
yoav-steinberg 2022-02-08 11:44:40 +02:00 committed by GitHub
parent 3c3e6cc1c7
commit b76016a948
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 88 additions and 10 deletions

View File

@ -432,7 +432,7 @@ static void redisProtocolToLuaType_Double(void *ctx, double d, const char *proto
* table is never a valid reply by proper commands, since the returned
* tables are otherwise always indexed by integers, never by strings. */
void luaPushError(lua_State *lua, char *error) {
lua_Debug dbg;
sds msg;
/* If debugging is active and in step mode, log errors resulting from
* Redis commands. */
@ -443,15 +443,21 @@ void luaPushError(lua_State *lua, char *error) {
lua_newtable(lua);
lua_pushstring(lua,"err");
/* Attempt to figure out where this function was called, if possible */
if(lua_getstack(lua, 1, &dbg) && lua_getinfo(lua, "nSl", &dbg)) {
sds msg = sdscatprintf(sdsempty(), "%s: %d: %s",
dbg.source, dbg.currentline, error);
/* There are two possible formats for the received `error` string:
* 1) "-CODE msg": in this case we remove the leading '-' since we don't store it as part of the lua error format.
* 2) "msg": in this case we prepend a generic 'ERR' code since all error statuses need some error code.
* We support format (1) so this function can reuse the error messages used in other places in redis.
* We support format (2) so it'll be easy to pass descriptive errors to this function without worrying about format.
*/
if (error[0] == '-')
msg = sdsnew(error+1);
else
msg = sdscatprintf(sdsempty(), "ERR %s", error);
/* Trim newline at end of string. If we reuse the ready-made Redis error objects (case 1 above) then we might
* have a newline that needs to be trimmed. In any case the lua Redis error table shouldn't end with a newline. */
msg = sdstrim(msg, "\r\n");
lua_pushstring(lua, msg);
sdsfree(msg);
} else {
lua_pushstring(lua, error);
}
lua_settable(lua,-3);
}

View File

@ -1400,3 +1400,75 @@ start_server {tags {"scripting"}} {
set _ {}
} {} {external:skip}
}
# Additional eval only tests
start_server {tags {"scripting"}} {
test "Consistent eval error reporting" {
r config set maxmemory 1
# Script aborted due to Redis state (OOM) should report script execution error with detailed internal error
assert_error {ERR Error running script (call to *): @user_script:*: OOM command not allowed when used memory > 'maxmemory'.} {
r eval {return redis.call('set','x','y')} 1 x
}
# redis.pcall() failure due to Redis state (OOM) returns lua error table with Redis error message without '-' prefix
assert_equal [
r eval {
local t = redis.pcall('set','x','y')
if t['err'] == "OOM command not allowed when used memory > 'maxmemory'." then
return 1
else
return 0
end
} 1 x
] 1
# Returning an error object from lua is handled as a valid RESP error result.
assert_error {OOM command not allowed when used memory > 'maxmemory'.} {
r eval { return redis.pcall('set','x','y') } 1 x
}
r config set maxmemory 0
# Script aborted due to error result of Redis command
assert_error {ERR Error running script (call to *): @user_script:*: ERR DB index is out of range} {
r eval {return redis.call('select',99)} 0
}
# redis.pcall() failure due to error in Redis command returns lua error table with redis error message without '-' prefix
assert_equal [
r eval {
local t = redis.pcall('select',99)
if t['err'] == "ERR DB index is out of range" then
return 1
else
return 0
end
} 0
] 1
# Script aborted due to scripting specific error state (write cmd with eval_ro) should report script execution error with detailed internal error
assert_error {ERR Error running script (call to *): @user_script:*: ERR Write commands are not allowed from read-only scripts.} {
r eval_ro {return redis.call('set','x','y')} 1 x
}
# redis.pcall() failure due to scripting specific error state (write cmd with eval_ro) returns lua error table with Redis error message without '-' prefix
assert_equal [
r eval_ro {
local t = redis.pcall('set','x','y')
if t['err'] == "ERR Write commands are not allowed from read-only scripts." then
return 1
else
return 0
end
} 1 x
] 1
} {} {cluster:skip}
test "LUA redis.error_reply API" {
assert_error {MY_ERR_CODE custom msg} {
r eval {return redis.error_reply("MY_ERR_CODE custom msg")} 0
}
}
test "LUA redis.status_reply API" {
r readraw 1
assert_equal [
r eval {return redis.status_reply("MY_OK_CODE custom msg")} 0
] {+MY_OK_CODE custom msg}
r readraw 0
}
}