[GitHub] [apisix] spacewander commented on a diff in pull request #7947: feat(ext-plugin-post-resp): support get response body by extra_info

[GitBox <gi...@apache.org>]

spacewander commented on code in PR #7947:
URL: https://github.com/apache/apisix/pull/7947#discussion_r977397141


##########
t/plugin/ext-plugin/extra-info.t:
##########
@@ -20,6 +20,7 @@ repeat_each(1);
 no_long_string();
 no_root_location();
 no_shuffle();
+log_level("info");

Review Comment:
   The default level is already info? https://github.com/apache/apisix/blob/a47d05a9a32db1ba7de6bda68c67cf0008f8442c/t/APISIX.pm#L24



##########
apisix/plugins/ext-plugin/init.lua:
##########
@@ -304,7 +305,31 @@ local function handle_extra_info(ctx, input)
         if err then
             core.log.error("failed to read request body: ", err)
         end
-
+    elseif info_type == extra_info.RespBody then
+        local ext_res = ctx.runner_ext_response
+        if ext_res then
+            local info = req:Info()
+            local respbody_req = extra_info_respbody.New()
+            respbody_req:Init(info.byte, info.pos)
+
+            local chunks = {}
+            local err = helper.response_reader(ext_res.body_reader, function (chunk, chunks)
+                -- When the upstream response is chunked type,
+                -- we will receive the complete response body
+                -- before sending it to the runner program
+                -- to reduce the number of RCP calls.

Review Comment:
   ```suggestion
                   -- to reduce the number of RPC calls.
   ```



##########
t/plugin/ext-plugin/extra-info.t:
##########
@@ -181,3 +182,156 @@ GET /hello
 --- error_code: 503
 --- error_log
 failed to receive RPC_HTTP_REQ_CALL: closed
+
+
+
+=== TEST 5: ask response body (not exist)

Review Comment:
   Let's add a test with empty response body



##########
apisix/plugins/ext-plugin/helper.lua:
##########
@@ -56,4 +56,27 @@ function _M.get_conf_token_cache_time()
 end
 
 
+function _M.response_reader(reader, callback, ...)
+    if not reader then
+        return "get response reader failed"
+    end
+
+    repeat
+        local chunk, read_err, cb_err
+        -- TODO: HEAD or 304

Review Comment:
   Could you use at least one sentence for this TODO? I can't recall why this comment is added.



-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org