From effa707e9db3e9d00fff06d45e2a5a81d3d55fa9 Mon Sep 17 00:00:00 2001 From: Madelyn Olson <34459052+madolson@users.noreply.github.com> Date: Thu, 14 Apr 2022 01:18:32 -0700 Subject: [PATCH] Fix incorrect error code for eval scripts and fix test error checking (#10575) By the convention of errors, there is supposed to be a space between the code and the name. While looking at some lua stuff I noticed that interpreter errors were not adding the space, so some clients will try to map the detailed error message into the error. We have tests that hit this condition, but they were just checking that the string "starts" with ERR. I updated some other tests with similar incorrect string checking. This isn't complete though, as there are other ways we check for ERR I didn't fix. Produces some fun output like: ``` # Errorstats errorstat_ERR:count=1 errorstat_ERRuser_script_1_:count=1 ``` --- src/eval.c | 2 +- src/function_lua.c | 2 +- tests/unit/acl-v2.tcl | 2 +- tests/unit/auth.tcl | 2 +- tests/unit/bitops.tcl | 4 ++-- tests/unit/geo.tcl | 18 +++++++++--------- tests/unit/introspection.tcl | 18 +++++++++--------- tests/unit/moduleapi/basics.tcl | 2 +- tests/unit/scripting.tcl | 4 ++-- tests/unit/type/hash.tcl | 18 +++++++++--------- tests/unit/type/incr.tcl | 10 +++++----- tests/unit/type/list.tcl | 2 +- tests/unit/type/stream.tcl | 8 ++++---- 13 files changed, 46 insertions(+), 46 deletions(-) diff --git a/src/eval.c b/src/eval.c index 22bcbdb73..4894b83d3 100644 --- a/src/eval.c +++ b/src/eval.c @@ -242,7 +242,7 @@ void scriptingInit(int setup) { " i = dbg.getinfo(3,'nSl')\n" " end\n" " if type(err) ~= 'table' then\n" - " err = {err='ERR' .. tostring(err)}" + " err = {err='ERR ' .. tostring(err)}" " end" " if i then\n" " err['source'] = i.source\n" diff --git a/src/function_lua.c b/src/function_lua.c index 8f21a1721..1e6363fd4 100644 --- a/src/function_lua.c +++ b/src/function_lua.c @@ -479,7 +479,7 @@ int luaEngineInitEngine() { " i = dbg.getinfo(3,'nSl')\n" " end\n" " if type(err) ~= 'table' then\n" - " err = {err='ERR' .. tostring(err)}" + " err = {err='ERR ' .. tostring(err)}" " end" " if i then\n" " err['source'] = i.source\n" diff --git a/tests/unit/acl-v2.tcl b/tests/unit/acl-v2.tcl index 12eb5a3be..500a7c729 100644 --- a/tests/unit/acl-v2.tcl +++ b/tests/unit/acl-v2.tcl @@ -173,7 +173,7 @@ start_server {tags {"acl external:skip"}} { assert_equal PONG [$r2 PING] assert_equal {} [$r2 get readwrite_str] - assert_error {ERR* not an integer *} {$r2 set readwrite_str bar ex get} + assert_error {ERR * not an integer *} {$r2 set readwrite_str bar ex get} assert_equal {OK} [$r2 set readwrite_str bar] assert_equal {bar} [$r2 get readwrite_str] diff --git a/tests/unit/auth.tcl b/tests/unit/auth.tcl index 4a4d7564c..26d125579 100644 --- a/tests/unit/auth.tcl +++ b/tests/unit/auth.tcl @@ -2,7 +2,7 @@ start_server {tags {"auth external:skip"}} { test {AUTH fails if there is no password configured server side} { catch {r auth foo} err set _ $err - } {ERR*any password*} + } {ERR *any password*} test {Arity check for auth command} { catch {r auth a b c} err diff --git a/tests/unit/bitops.tcl b/tests/unit/bitops.tcl index ec08a060d..b3a0b52b5 100644 --- a/tests/unit/bitops.tcl +++ b/tests/unit/bitops.tcl @@ -133,12 +133,12 @@ start_server {tags {"bitops"}} { test {BITCOUNT syntax error #1} { catch {r bitcount s 0} e set e - } {ERR*syntax*} + } {ERR *syntax*} test {BITCOUNT syntax error #2} { catch {r bitcount s 0 1 hello} e set e - } {ERR*syntax*} + } {ERR *syntax*} test {BITCOUNT regression test for github issue #582} { r del foo diff --git a/tests/unit/geo.tcl b/tests/unit/geo.tcl index e6afb211b..1c7564430 100644 --- a/tests/unit/geo.tcl +++ b/tests/unit/geo.tcl @@ -193,14 +193,14 @@ start_server {tags {"geo"}} { r geoadd nyc xx nx -73.9454966 40.747533 "lic market" } err set err - } {ERR*syntax*} + } {ERR *syntax*} test {GEOADD update with invalid option} { catch { r geoadd nyc ch xx foo -73.9454966 40.747533 "lic market" } err set err - } {ERR*syntax*} + } {ERR *syntax*} test {GEOADD invalid coordinates} { catch { @@ -229,27 +229,27 @@ start_server {tags {"geo"}} { test {GEOSEARCH FROMLONLAT and FROMMEMBER cannot exist at the same time} { catch {r geosearch nyc fromlonlat -73.9798091 40.7598464 frommember xxx bybox 6 6 km asc} e set e - } {ERR*syntax*} + } {ERR *syntax*} test {GEOSEARCH FROMLONLAT and FROMMEMBER one must exist} { catch {r geosearch nyc bybox 3 3 km asc desc withhash withdist withcoord} e set e - } {ERR*exactly one of FROMMEMBER or FROMLONLAT*} + } {ERR *exactly one of FROMMEMBER or FROMLONLAT*} test {GEOSEARCH BYRADIUS and BYBOX cannot exist at the same time} { catch {r geosearch nyc fromlonlat -73.9798091 40.7598464 byradius 3 km bybox 3 3 km asc} e set e - } {ERR*syntax*} + } {ERR *syntax*} test {GEOSEARCH BYRADIUS and BYBOX one must exist} { catch {r geosearch nyc fromlonlat -73.9798091 40.7598464 asc desc withhash withdist withcoord} e set e - } {ERR*exactly one of BYRADIUS and BYBOX*} + } {ERR *exactly one of BYRADIUS and BYBOX*} test {GEOSEARCH with STOREDIST option} { catch {r geosearch nyc fromlonlat -73.9798091 40.7598464 bybox 6 6 km asc storedist} e set e - } {ERR*syntax*} + } {ERR *syntax*} test {GEORADIUS withdist (sorted)} { r georadius nyc -73.9798091 40.7598464 3 km withdist asc @@ -274,12 +274,12 @@ start_server {tags {"geo"}} { test {GEORADIUS with ANY but no COUNT} { catch {r georadius nyc -73.9798091 40.7598464 10 km ANY ASC} e set e - } {ERR*ANY*requires*COUNT*} + } {ERR *ANY*requires*COUNT*} test {GEORADIUS with COUNT but missing integer argument} { catch {r georadius nyc -73.9798091 40.7598464 10 km COUNT} e set e - } {ERR*syntax*} + } {ERR *syntax*} test {GEORADIUS with COUNT DESC} { r georadius nyc -73.9798091 40.7598464 10 km COUNT 2 DESC diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl index c530a31d3..339d4c731 100644 --- a/tests/unit/introspection.tcl +++ b/tests/unit/introspection.tcl @@ -23,9 +23,9 @@ start_server {tags {"introspection"}} { assert_error "ERR wrong number of arguments for 'client|kill' command" {r client kill} assert_error "ERR syntax error*" {r client kill id 10 wrong_arg} - assert_error "ERR*greater than 0*" {r client kill id str} - assert_error "ERR*greater than 0*" {r client kill id -1} - assert_error "ERR*greater than 0*" {r client kill id 0} + assert_error "ERR *greater than 0*" {r client kill id str} + assert_error "ERR *greater than 0*" {r client kill id -1} + assert_error "ERR *greater than 0*" {r client kill id 0} assert_error "ERR Unknown client type*" {r client kill type wrong_type} @@ -409,11 +409,11 @@ start_server {tags {"introspection"}} { } test {CONFIG SET duplicate configs} { - assert_error "ERR*duplicate*" {r config set maxmemory 10000001 maxmemory 10000002} + assert_error "ERR *duplicate*" {r config set maxmemory 10000001 maxmemory 10000002} } test {CONFIG SET set immutable} { - assert_error "ERR*immutable*" {r config set daemonize yes} + assert_error "ERR *immutable*" {r config set daemonize yes} } test {CONFIG GET hidden configs} { @@ -448,8 +448,8 @@ start_server {tags {"introspection"}} { start_server {tags {"introspection external:skip"} overrides {enable-protected-configs {no} enable-debug-command {no}}} { test {cannot modify protected configuration - no} { - assert_error "ERR*protected*" {r config set dir somedir} - assert_error "ERR*DEBUG command not allowed*" {r DEBUG HELP} + assert_error "ERR *protected*" {r config set dir somedir} + assert_error "ERR *DEBUG command not allowed*" {r DEBUG HELP} } {} {needs:debug} } @@ -464,8 +464,8 @@ start_server {config "minimal.conf" tags {"introspection external:skip"} overrid if {$myaddr != "" && ![string match {127.*} $myaddr]} { # Non-loopback client should fail set r2 [get_nonloopback_client] - assert_error "ERR*protected*" {$r2 config set dir somedir} - assert_error "ERR*DEBUG command not allowed*" {$r2 DEBUG HELP} + assert_error "ERR *protected*" {$r2 config set dir somedir} + assert_error "ERR *DEBUG command not allowed*" {$r2 DEBUG HELP} } } {} {needs:debug} } diff --git a/tests/unit/moduleapi/basics.tcl b/tests/unit/moduleapi/basics.tcl index b858c344a..040d9eb3c 100644 --- a/tests/unit/moduleapi/basics.tcl +++ b/tests/unit/moduleapi/basics.tcl @@ -36,6 +36,6 @@ start_server {tags {"modules"}} { start_server {tags {"modules external:skip"} overrides {enable-module-command no}} { test {module command disabled} { - assert_error "ERR*MODULE command not allowed*" {r module load $testmodule} + assert_error "ERR *MODULE command not allowed*" {r module load $testmodule} } } \ No newline at end of file diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl index d9729b7bd..02eecefd7 100644 --- a/tests/unit/scripting.tcl +++ b/tests/unit/scripting.tcl @@ -447,12 +447,12 @@ start_server {tags {"scripting"}} { test {Globals protection reading an undeclared global variable} { catch {run_script {return a} 0} e set e - } {ERR*attempted to access * global*} + } {ERR *attempted to access * global*} test {Globals protection setting an undeclared global*} { catch {run_script {a=10} 0} e set e - } {ERR*attempted to create global*} + } {ERR *attempted to create global*} test {Test an example script DECR_IF_GT} { set decr_if_gt { diff --git a/tests/unit/type/hash.tcl b/tests/unit/type/hash.tcl index 7e3b3e2ca..ae5677383 100644 --- a/tests/unit/type/hash.tcl +++ b/tests/unit/type/hash.tcl @@ -507,8 +507,8 @@ start_server {tags {"hash"}} { catch {r hincrby smallhash str 1} smallerr catch {r hincrby bighash str 1} bigerr set rv {} - lappend rv [string match "ERR*not an integer*" $smallerr] - lappend rv [string match "ERR*not an integer*" $bigerr] + lappend rv [string match "ERR *not an integer*" $smallerr] + lappend rv [string match "ERR *not an integer*" $bigerr] } {1 1} test {HINCRBY fails against hash value with spaces (right)} { @@ -517,8 +517,8 @@ start_server {tags {"hash"}} { catch {r hincrby smallhash str 1} smallerr catch {r hincrby bighash str 1} bigerr set rv {} - lappend rv [string match "ERR*not an integer*" $smallerr] - lappend rv [string match "ERR*not an integer*" $bigerr] + lappend rv [string match "ERR *not an integer*" $smallerr] + lappend rv [string match "ERR *not an integer*" $bigerr] } {1 1} test {HINCRBY can detect overflows} { @@ -579,8 +579,8 @@ start_server {tags {"hash"}} { catch {r hincrbyfloat smallhash str 1} smallerr catch {r hincrbyfloat bighash str 1} bigerr set rv {} - lappend rv [string match "ERR*not*float*" $smallerr] - lappend rv [string match "ERR*not*float*" $bigerr] + lappend rv [string match "ERR *not*float*" $smallerr] + lappend rv [string match "ERR *not*float*" $bigerr] } {1 1} test {HINCRBYFLOAT fails against hash value with spaces (right)} { @@ -589,15 +589,15 @@ start_server {tags {"hash"}} { catch {r hincrbyfloat smallhash str 1} smallerr catch {r hincrbyfloat bighash str 1} bigerr set rv {} - lappend rv [string match "ERR*not*float*" $smallerr] - lappend rv [string match "ERR*not*float*" $bigerr] + lappend rv [string match "ERR *not*float*" $smallerr] + lappend rv [string match "ERR *not*float*" $bigerr] } {1 1} test {HINCRBYFLOAT fails against hash value that contains a null-terminator in the middle} { r hset h f "1\x002" catch {r hincrbyfloat h f 1} err set rv {} - lappend rv [string match "ERR*not*float*" $err] + lappend rv [string match "ERR *not*float*" $err] } {1} test {HSTRLEN against the small hash} { diff --git a/tests/unit/type/incr.tcl b/tests/unit/type/incr.tcl index b6aaa1112..b7446850e 100644 --- a/tests/unit/type/incr.tcl +++ b/tests/unit/type/incr.tcl @@ -111,21 +111,21 @@ start_server {tags {"incr"}} { r set novar " 11" catch {r incrbyfloat novar 1.0} err format $err - } {ERR*valid*} + } {ERR *valid*} test {INCRBYFLOAT fails against key with spaces (right)} { set err {} r set novar "11 " catch {r incrbyfloat novar 1.0} err format $err - } {ERR*valid*} + } {ERR *valid*} test {INCRBYFLOAT fails against key with spaces (both)} { set err {} r set novar " 11 " catch {r incrbyfloat novar 1.0} err format $err - } {ERR*valid*} + } {ERR *valid*} test {INCRBYFLOAT fails against a key holding a list} { r del mylist @@ -146,7 +146,7 @@ start_server {tags {"incr"}} { # p.s. no way I can force NaN to test it from the API because # there is no way to increment / decrement by infinity nor to # perform divisions. - } {ERR*would produce*} + } {ERR *would produce*} } test {INCRBYFLOAT decrement} { @@ -159,7 +159,7 @@ start_server {tags {"incr"}} { r setrange foo 2 2 catch {r incrbyfloat foo 1} err format $err - } {ERR*valid*} + } {ERR *valid*} test {No negative zero} { r del foo diff --git a/tests/unit/type/list.tcl b/tests/unit/type/list.tcl index 8cff56c6a..ebc869e23 100644 --- a/tests/unit/type/list.tcl +++ b/tests/unit/type/list.tcl @@ -1173,7 +1173,7 @@ foreach {pop} {BLPOP BLMPOP_LEFT} { test "$pop: with negative timeout" { set rd [redis_deferring_client] bpop_command $rd $pop blist1 -1 - assert_error "ERR*is negative*" {$rd read} + assert_error "ERR *is negative*" {$rd read} $rd close } diff --git a/tests/unit/type/stream.tcl b/tests/unit/type/stream.tcl index bd689cd29..83d29bbbf 100644 --- a/tests/unit/type/stream.tcl +++ b/tests/unit/type/stream.tcl @@ -770,7 +770,7 @@ start_server {tags {"stream xsetid"}} { catch {r XSETID mystream "1-1"} err r XADD mystream MAXLEN 0 * a b set err - } {ERR*smaller*} + } {ERR *smaller*} test {XSETID cannot SETID on non-existent key} { catch {r XSETID stream 1-1} err @@ -790,7 +790,7 @@ start_server {tags {"stream xsetid"}} { test {XSETID errors on negstive offset} { catch {r XSETID stream 1-1 ENTRIESADDED -1 MAXDELETEDID 0-0} err set _ $err - } {ERR*must be positive} + } {ERR *must be positive} test {XSETID cannot set the maximal tombstone with larger ID} { r DEL x @@ -799,7 +799,7 @@ start_server {tags {"stream xsetid"}} { catch {r XSETID x "1-0" ENTRIESADDED 1 MAXDELETEDID "2-0" } err r XADD mystream MAXLEN 0 * a b set err - } {ERR*smaller*} + } {ERR *smaller*} test {XSETID cannot set the offset to less than the length} { r DEL x @@ -808,7 +808,7 @@ start_server {tags {"stream xsetid"}} { catch {r XSETID x "1-0" ENTRIESADDED 0 MAXDELETEDID "0-0" } err r XADD mystream MAXLEN 0 * a b set err - } {ERR*smaller*} + } {ERR *smaller*} } start_server {tags {"stream offset"}} {