-
Notifications
You must be signed in to change notification settings - Fork 24.5k
Description
When running a LUA script via eval we may get an Redis error response (a RESP error line starting with -) for different reasons:
- We executed a Redis command with
redis.call()which returned an error. - We returned a result of a call
redis.pcall()which itself returned an error. - We failed to execute a command via
redis.call()because of some state (for example write command withEVAL_ROduring OOM condition). - We returned a result of failed attempt to execute a command via
redis.pcall()because of some state (for example write command withEVAL_ROduring OOM condition).
Each of these returns the error differently and in cases 3,4 above the format seems a bit arbitrary based on what type of error state was encountered.
Details:
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'.
5: eval "return redis.pcall('set','x','y')" 0
6: -@user_script: 1: -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.
13: eval_ro "return redis.pcall('set','x','y')" 0
14: -@user_script: 1: Write commands are not allowed from read-only scripts.
Issues:
-
In line 4 (and 12)
redis.call()failed because of pre-existing state (OOM/ro). Script execution is aborted with generic error (-ERR Error runnning script) wrapping the internal error (-OOM...). The wrapper includes a "stack trace"(@user_script:1). The internal error is also wrapped in a stack trace (this time with a space before the line number@user_script: 1). We should avoid the double stack trace. -
In line 4 the internal error is formatted like a valid Redis error response (
-OOM), but in line 12 we have a similar call that fails before of attempting to call aWRITEcommand fromeval_ro. This time the internal error is just a string with error code or status marker (Write commands are not...). This is inconsistent. -
In line 6 we return the error object we received from a failed
redis.pcall()because of an OOM state. This isn't prefixed with any error code, which seems very unorthodox and should be fixed. -
In line 8 we abort the script with an error because of an error in the command executed by a
redis.call()(select 99). The error is wrapped generically with a "stack trace" and seems fine but this time the wrapped error's leading-was trimmed becoming inconsistent with line 4.
In addition to this here:
Lines 447 to 454 in 857dc5b
| if(lua_getstack(lua, 1, &dbg) && lua_getinfo(lua, "nSl", &dbg)) { | |
| sds msg = sdscatprintf(sdsempty(), "%s: %d: %s", | |
| dbg.source, dbg.currentline, error); | |
| lua_pushstring(lua, msg); | |
| sdsfree(msg); | |
| } else { | |
| lua_pushstring(lua, error); | |
| } |
There's code (which in practice never executes) which returns the error string unwrapped by the "stack trace" if it fails to get it. In such a case we might return a double "-" sign or prepend the "-" sign to some random text like
-Write commands are not allowed....