Suboptimal Lua representation of Redis errors about redis HOT 7 CLOSED

From a user's perspective, I have to let you know that I had to find this issue to determine what the proper check for error should be after using pcall.

from redis.

Hmmm...You bring up a good point about clean error codes.

This may seem a tad hackish, but maybe when an error returns from the Lua script to Redis, Redis could check to see if it matches the format of a Redis error (i.e. starts with an all-uppercase word). If it does, it returns it as an error message to the client as is. Otherwise, it prepends ERR Error running script: and sends it back as it does now. That way, errors generated by calling Redis commands are returned just as they would be if the user called the command directly, but other errors are clearly marked as coming from the script.

Though as for the issue of "the possibility that bad scripts will not check for exceptions and will silently return OK even if the operation was not performed correctly," part of what I was trying to say was that Redis errors are things that go wrong very rarely, and usually the script cannot continue when they happen. It is likely that most of the error handling code in Redis scripts using the current error system would look like:

local result = redis.call("command", "arg", "arg")
if type(result) == 'table' and result.err then
  return result
end

Which is a lot of boilerplate, especially when you are using the return value of redis.call in an expression. Raising Redis errors as Lua errors and transparently sending them to the client makes the boilerplate unnecessary for most calls, and in the less common case where errors are recoverable, you can use:

local ok, result = pcall(redis.call, "command", "arg", "arg")
if not ok then
  -- recover from the error
end

If this turns out to be commonly used, perhaps a redis.pcall could be added that just pcalls redis.call.

from redis.

I don't agree, and IMHO the fact that a part of Lua programmers see the current approach as an error is that they think at the Redis scripting API as a Lua Redis client.

In a Lua Redis client you of course want to raise an error when a protocol error is returned. However Redis scripting support is instead created in order to implement commands in Redis. In such a case you don't want to raise errors as you are somewhat programming at a lower level of abstraction, what you have when you execute a Redis command is a reply, that is an object that represents what Redis produced as a result for a command. Errors are non exceptions, it's just the Redis protocol, so why I would raise an exception? The Lua script is required to make sure that the semantics makes sense, and to return a meaningful return value only if the operation performed ended in the intended way.

Another more practical example. I want to do a small script performing an operation against a List.
If there is a type mismatch as the user calls the script with a key holding a Set I don't want a Lua stack trace as a result, but I want a good error in the form of "-ERR operation against the wrong data type" or alike.
If we raise an error most scripts will just let Lua exception handle that, and we'll have bad error codes.

I agree that there is the possibility that bad scripts will not check for exceptions and will silently return OK even if the operation was not performed correctly, but there is no cure for bad code, and raising an exception will not save us from bad scripts anyway.

So in the end: it is a design issue, there is not right/wrong way, and my personal design taste tells me that at this level of abstraction what I want back is a reply object that represents what Redis returned. This is why I will take it as it is, but at the same time can't say you are wrong.

Thanks for the well thought and explained request.

from redis.

@leafstorm: Want to share with you that I changed my mind about that, and finally implemented two different kind of calls:

Redis.call() will raise a Lua error on Redis errors, if that errors are at "top level" (so no error raised if you get just one error into a multi-bulk stuff).

Redis.pcall() instead will return the error object, like the old .call method used to do.

I hope this new solution is a better compromise between all the different requirements.

Salvatore

from redis.

Definitely an improvement, thanks. I was still a bit unsure about returning {err = message} instead of the standard Lua nil, message, but upon further reflection, this makes sense because it means that you don't have to use a separate representation in multibulks and in top-level calls.

from redis.

@offero the eval documentation includes a note about call/pcall.
Did you just not find it there? Did you expect it else where? Have you any suggestions on how to make it more clear?

from redis.

@badboy I've just ended up here trying to work out how to catch the redis.pcall error (as @offero described). I think the eval documentation could do with some example code - but that's probably my problem because I'm not too familiar with Lua, so I didn't understand the reference to a table until I saw @leafstorm 's example code above. The pure Lua examples I found elsewhere didn't work because they're coded as per @leafstorm 's 2nd example.

from redis.