Copilot commented on code in PR #13461:
URL: https://github.com/apache/apisix/pull/13461#discussion_r3332829869
##########
apisix/plugins/ai-transport/http.lua:
##########
@@ -20,13 +20,19 @@
local core = require("apisix.core")
local http = require("resty.http")
+local rapidjson = require("rapidjson")
+local getmetatable = getmetatable
local ngx_now = ngx.now
local pairs = pairs
local ipairs = ipairs
+local pcall = pcall
local type = type
local str_lower = string.lower
+local tostring = tostring
local _M = {}
+local rapidjson_encode_opts = {sort_keys = true}
+local rapidjson_null = rapidjson.null
Review Comment:
`require("rapidjson")` happens at module load time, so if the module is
missing or fails to load, this transport module will error before `encode_body`
can fall back to cjson. To match the intended “never fail the request on
rapidjson issues” behavior, load rapidjson defensively and treat it as optional
at runtime.
##########
apisix/plugins/ai-transport/http.lua:
##########
@@ -65,6 +71,50 @@ function _M.construct_forward_headers(ext_opts_headers, ctx)
end
+local function to_rapidjson_value(data)
+ if data == core.json.null then
+ return rapidjson_null
+ end
+
+ if type(data) ~= "table" then
+ return data
+ end
+
+ if getmetatable(data) == core.json.array_mt then
+ local arr = {}
+ for i, v in ipairs(data) do
+ arr[i] = to_rapidjson_value(v)
+ end
+ return rapidjson.array(arr)
+ end
+
+ local obj = {}
+ for k, v in pairs(data) do
+ obj[k] = to_rapidjson_value(v)
+ end
+ return obj
+end
+
+
+local function rapidjson_encode(data)
+ return rapidjson.encode(data, rapidjson_encode_opts)
+end
+
+
+local function encode_body(body)
+ local ok, encoded, err = pcall(rapidjson_encode, to_rapidjson_value(body))
+ if ok and encoded then
+ return encoded
+ end
+
+ core.log.error("failed to encode AI request body with rapidjson: ",
+ ok and (err or "unknown") or tostring(encoded),
+ ", fallback to cjson; LLM cache hit rate may decrease")
+
+ return core.json.encode(body)
+end
Review Comment:
`pcall(rapidjson_encode, to_rapidjson_value(body))` does not catch errors
thrown by `to_rapidjson_value(body)` because arguments are evaluated before
`pcall` executes. If conversion throws, the request will error instead of
falling back. Also, if rapidjson failed to load, `encode_body` should
immediately fall back to `core.json.encode`.
##########
ci/install-lua-rapidjson.sh:
##########
@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+RAPIDJSON_VERSION="${RAPIDJSON_VERSION:-0.7.2}"
+ROCKSPEC_VERSION="${ROCKSPEC_VERSION:-0.7.2-1}"
+LUAROCKS_BIN="${LUAROCKS:-luarocks}"
+TREE="${1:-}"
+
+if [ -n "${TREE}" ]; then
+ mkdir -p "${TREE}"
+ TREE="$(cd "${TREE}" && pwd)"
+fi
+
+workdir="$(mktemp -d)"
+trap 'rm -rf "${workdir}"' EXIT
+
+cd "${workdir}"
+"${LUAROCKS_BIN}" unpack rapidjson "${RAPIDJSON_VERSION}"
+cd "rapidjson-${ROCKSPEC_VERSION}/lua-rapidjson"
+
+# lua-rapidjson enables -march=native by default, which can leak CI CPU
+# features into the shipped rapidjson.so.
+sed -i '/add_compile_options(-march=native)/d' CMakeLists.txt
+
+if [ -n "${TREE}" ]; then
+ "${LUAROCKS_BIN}" make "rapidjson-${ROCKSPEC_VERSION}.rockspec"
--tree="${TREE}" --local
+else
+ "${LUAROCKS_BIN}" make "rapidjson-${ROCKSPEC_VERSION}.rockspec"
+fi
Review Comment:
The `sed` patch only matches an exact `add_compile_options(-march=native)`
line; if upstream changes formatting or combines flags, `-march=native` may
remain and reintroduce the portability crash risk. Also, `luarocks make ...
--tree=... --local` is ambiguous (and may ignore the intended tree), which
could cause later installs to rebuild rapidjson unpatched.
--
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]
