moonming commented on PR #14012:
URL: https://github.com/apache/apisix/pull/14012#issuecomment-6792927793
Thanks for the fix — the nil concat is real, and it's worse than a log line:
since the batch processor doesn't pcall the callback, a throw skips both retry
and the `processed_entries` accounting, so with the default
`max_pending_entries` (8192, since 3.18) the worker eventually discards all
splunk logs until reload, even after the endpoint recovers. I reproduced this
on 3.19.0.
Two remaining cases still throw with this patch: `{"text": null}`
(cjson.null is truthy userdata) and `{"message": {...}}` (table). Also, for
non-JSON bodies (e.g. an HTML 502 from a proxy) the status code is now dropped.
Suggest:
if type(body) == "table" and type(body.text) == "string" then
return false, "failed to send splunk, " .. body.text
end
return false, "failed to send splunk, http status code: " .. res.status
.. ", body: " .. (res.body or "")
This keeps the existing `Invalid authorization` assertion unchanged. A test
case for `{"text": null}` would be good too. #14192 (draft) makes the batch
processor pcall the callback, so this class of bug can't leak entries in any
logger.
--
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]
