blaisewang commented on PR #13763: URL: https://github.com/apache/apisix/pull/13763#issuecomment-5191101323
> Two P2 issues need to be addressed before merge: > > 1. **Response reporting can fail silently, and the tests do not verify the core behavior.** `ngx.timer.at` does not consume callback return values, so connect/send/receive failures returned by `socket.do_socket` are neither logged nor measured. Tests 18-21 only verify configuration acceptance and the client response; they do not prove that the mock WAF receives the response status, headers, and truncated body, or that ignored content types are skipped. Please add observable error handling and an end-to-end assertion with bounded polling for the asynchronous report. > 2. **The required exact-head CI is failing.** The `t/plugin/[a-k]*.t` job is red in `kafka-logger.t` TEST 18 because the expected `partition_id: 1` and `partition_id: 2` log entries are missing. This appears outside the files changed by this PR, but the required check still needs to pass before merge. Please rerun it and investigate if the failure is reproducible: [failed job](https://github.com/apache/apisix/actions/runs/30528555117/job/91623243841). 1. Fixed in lua-resty-t1k 1.2.1 — `ngx.timer.at` discarded the callback's return values, so `do_socket` failures are now logged at error level, and the dependency is bumped here. Added `t/plugin/chaitin-waf-log-resp.t`, which asserts the report is actually sent, that a 4096-byte response is truncated to 1024 for reporting while the client still gets all 4096, that ignored content types and `log_resp: false` skip it, and that delivery failures are logged. 2. Unrelated to this PR. CI should pass on rerun. -- 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]
