diff --git a/src/apisix/plugins/bk-error-wrapper.lua b/src/apisix/plugins/bk-error-wrapper.lua index 1135c3e..8e02d57 100644 --- a/src/apisix/plugins/bk-error-wrapper.lua +++ b/src/apisix/plugins/bk-error-wrapper.lua @@ -69,9 +69,14 @@ end --- - 502 + failed to connect to upstream: the upstream port is not listened --- - 504 + failed to connect to upstream: the upstream port is listened, but the handshake timeout --- usually network problem +--- - 502 + cannot read header from upstream: upstream prematurely closed connection while reading +--- response header from upstream +--- - 504 + cannot read header from upstream: upstream timed out (110: Connection timed out) +--- while reading response header from upstream ---@param ctx apisix.Context ---@return string,string|nil @the specific phase and error message local function _get_upstream_error_msg(ctx) + -- FIXME: add ngx.status as parameter, return specific error message in comment above if not ctx.var.upstream_connect_time then -- 握手失败 return proxy_phases.CONNECTING, "failed to connect to upstream" -- note: 此处删掉对$upstream_bytes_sent判断的原因是 @@ -109,13 +114,30 @@ function _M.header_filter(conf, ctx) -- luacheck: no unused ctx.var.proxy_error = "1" end + -- if not ctx.var.upstream_status, means from apisix internal or the plugin return + + -- 2024-10-10 封装导致非蓝鲸插件例如 fault-injection 返回非 200 时 response body 被吞掉 + -- 注释掉之后 + -- 1. 插件返回的非 200 不会存在 ctx.var.bk_apigw_error + -- 2. apisix 和 openresty 默认错误不会被封装, 将返回原始 body -- apisix or openresty default error - if not ctx.var.bk_apigw_error and ngx.status >= ngx.HTTP_BAD_REQUEST then - -- wrap and generate a bk_apigw_error - local error = errorx.new_default_error_with_status(ngx.status) - -- after set this, the body_filter will be called - ctx.var.bk_apigw_error = error + -- or it's other plugin return non-200 status + -- if not ctx.var.bk_apigw_error and ngx.status >= ngx.HTTP_BAD_REQUEST then + -- -- wrap and generate a bk_apigw_error + -- local error = errorx.new_default_error_with_status(ngx.status) + -- -- after set this, the body_filter will be called + -- ctx.var.bk_apigw_error = error + -- end + + -- upstream status 报错封装成网关的报错 + if upstream_error_msg then + if not ctx.var.bk_apigw_error and ngx.status >= ngx.HTTP_BAD_REQUEST then + -- wrap and generate a bk_apigw_error + local error = errorx.new_default_error_with_status(ngx.status) + -- after set this, the body_filter will be called + ctx.var.bk_apigw_error = error + end end local apigw_error = ctx.var.bk_apigw_error @@ -146,6 +168,7 @@ local function extract_error_info_from_body(body) if pl_types.is_empty(body) then return nil end + -- FIXME: should use the raw body as error message? -- openresty default error message for non-200 status, for example -- \r\n404 Not Found<\/title>... diff --git a/src/apisix/tests/test-bk-error-wrapper.lua b/src/apisix/tests/test-bk-error-wrapper.lua index f8b5caa..60db051 100644 --- a/src/apisix/tests/test-bk-error-wrapper.lua +++ b/src/apisix/tests/test-bk-error-wrapper.lua @@ -98,7 +98,7 @@ describe( ) context( - "apisix plugins error", function() + "apisix plugins error, will use raw status and response body", function() before_each( function() @@ -110,48 +110,30 @@ describe( ) it( - "apisix plugins error", function() + "apisix plugins return 404", function() ngx.status = 404 plugin.header_filter(nil, ctx) + assert.is_nil(ctx.var.bk_apigw_error) - local bk_apigw_error = errorx.new_api_not_found() - - assert.is_equal(bk_apigw_error.status, ctx.var.bk_apigw_error.status) - assert.is_equal(bk_apigw_error.error.code, ctx.var.bk_apigw_error.error.code) + -- local bk_apigw_error = errorx.new_api_not_found() - assert.stub(response.clear_header_as_body_modified).was.called() - assert.stub(response.set_header) - .was_called_with("Content-Type", "application/json; charset=utf-8") - assert.stub(response.set_header).was_called_with( - "X-Bkapi-Error-Code", tostring(bk_apigw_error.error.code) - ) - assert.stub(response.set_header).was_called_with( - "X-Bkapi-Error-Message", bk_apigw_error.error.message - ) - assert.is_nil(ctx.var.proxy_phase) - end - ) - - it( - "apisix plugins error but do'not need dealing with", function() - ngx.status = 409 - plugin.header_filter(nil, ctx) - assert.is_not_nil(ctx.var.bk_apigw_error) + -- assert.is_equal(bk_apigw_error.status, ctx.var.bk_apigw_error.status) + -- assert.is_equal(bk_apigw_error.error.code, ctx.var.bk_apigw_error.error.code) - local bk_apigw_error = errorx.new_unkonwon_error(409) + assert.stub(response.clear_header_as_body_modified).was_not_called() + -- assert.stub(response.set_header) + -- .was_called_with("Content-Type", "application/json; charset=utf-8") + -- assert.stub(response.set_header).was_called_with( + -- "X-Bkapi-Error-Code", tostring(bk_apigw_error.error.code) + -- ) + -- assert.stub(response.set_header).was_called_with( + -- "X-Bkapi-Error-Message", bk_apigw_error.error.message + -- ) - assert.stub(response.clear_header_as_body_modified).was_called() - assert.stub(response.set_header) - .was_called_with("Content-Type", "application/json; charset=utf-8") - assert.stub(response.set_header).was_called_with( - "X-Bkapi-Error-Code", tostring(bk_apigw_error.error.code) - ) - assert.stub(response.set_header).was_called_with( - "X-Bkapi-Error-Message", bk_apigw_error.error.message - ) assert.is_nil(ctx.var.proxy_phase) end ) + end ) @@ -509,6 +491,20 @@ describe( assert.stub(core.json.encode).called(0) end ) + + it( + "apisix plugin return status not 200", function() + ngx.status = 502 + ngx.arg[1] = "it's 502" + ngx.arg[2] = true + plugin.header_filter(nil, ctx) + plugin.body_filter(nil, ctx) + + assert.stub(core.json.encode).called(0) + assert.equal("it's 502", ngx.arg[1]) + assert.equal(true, ngx.arg[2]) + end + ) end ) end