nic-6443 commented on code in PR #13212:
URL: https://github.com/apache/apisix/pull/13212#discussion_r3071781927
##########
apisix/plugins/limit-count/limit-count-local.lua:
##########
@@ -66,16 +67,21 @@ function _M.new(plugin_name, limit, window)
end
function _M.incoming(self, key, commit, conf, cost)
- local delay, remaining = self.limit_count:incoming(key, commit, cost)
+ local delay, consumed_or_err = self.limit_count:incoming(key, commit, cost)
local reset
- if remaining == self.limit - cost then
+ local remaining_or_err = consumed_or_err
+ if type(consumed_or_err) == "number" then
+ remaining_or_err = self.limit - consumed_or_err
+ end
+
+ if remaining_or_err == self.limit - cost then
reset = set_endtime(self, key, self.window)
else
reset = read_reset(self, key)
end
Review Comment:
The reset-header dict write during peek is intentional and matches the
v1.0.0 behavior — with the old `commit=false, cost=1`, `remaining == self.limit
- 1` was also true on the first peek, triggering `set_endtime()` the same way.
Guarding with `commit == true` would cause the access-phase dry_run to
return `reset=0` (since the key doesn't exist yet). Since ai-rate-limiting sets
the `X-RateLimit-Reset` header in the access phase (not log phase), clients
would see `Reset: 0` instead of a meaningful window time. The current behavior
provides the correct reset countdown to clients.
The dict entry is for the same key that the subsequent real commit will
create anyway, so there's no net growth — just slightly earlier creation.
--
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]
