nic-6443 commented on code in PR #12751:
URL: https://github.com/apache/apisix/pull/12751#discussion_r3085621365
##########
apisix/plugins/limit-count/limit-count-redis.lua:
##########
@@ -52,37 +43,79 @@ function _M.new(plugin_name, limit, window, conf)
return setmetatable(self, mt)
end
-function _M.incoming(self, key, cost)
+
+local function release_connection(red, conf)
+ local ok, err = red:set_keepalive(conf.redis_keepalive_timeout,
conf.redis_keepalive_pool)
+ if ok then
+ return true
+ end
Review Comment:
Errors from this timer callback are silently swallowed. In OpenResty, return
values from `ngx.timer.at` callbacks are discarded — nobody reads `return red,
err` or `return nil, keepalive_err` or `return res, incoming_err`.
For `ai-rate-limiting`, the log phase is where actual LLM token usage gets
deducted from the counter. If Redis is unreachable, the deduction silently
fails with **zero visibility** — tokens are consumed but never counted, and
operators have no indication.
The caller in `init.lua` only catches `ngx.timer.at()` creation failure
(extremely rare), not the actual Redis operation failure inside the timer.
Same issue exists in `limit-count-redis-cluster.lua`'s
`log_phase_incoming_thread`.
Should add `core.log.error(...)` for each error path inside the callback:
```lua
local red, err = redis.new(conf)
if not red then
core.log.error("failed to create redis in log phase: ", err)
return
end
...
if incoming_err then
core.log.error("failed to deduct tokens in log phase: ", incoming_err)
end
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
