Skip to content

Commit

Permalink
refactor(pdk): more friendly error msg on arguments checking (#13224)
Browse files Browse the repository at this point in the history 
The function `assert` doesn't show the caller of the PDK function when failed to validate incoming arguments. Invoking function `error` when unable to validate an argument can get a better error message.

KAG-4679
  • Loading branch information
@ADD-SP
ADD-SP authored Jun 19, 2024
1 parent a3f5410 commit 65201e8
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 21 deletions.
49 changes: 35 additions & 14 deletions kong/pdk/private/rate_limiting.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,34 @@ local max_fields_n = 4
local _M = {}


local function _validate_key(key, arg_n, func_name)
local typ = type(key)
if typ ~= "string" then
local msg = string.format(
"arg #%d `key` for function `%s` must be a string, got %s",
arg_n,
func_name,
typ
)
error(msg, 3)
end
end


local function _validate_value(value, arg_n, func_name)
local typ = type(value)
if typ ~= "number" and typ ~= "string" then
local msg = string.format(
"arg #%d `value` for function `%s` must be a string or a number, got %s",
arg_n,
func_name,
typ
)
error(msg, 3)
end
end


local function _has_rl_ctx(ngx_ctx)
return ngx_ctx.__rate_limiting_context__ ~= nil
end
Expand Down Expand Up @@ -42,27 +70,20 @@ end


function _M.store_response_header(ngx_ctx, key, value)
assert(
type(key) == "string",
"arg #2 `key` for function `store_response_header` must be a string"
)

local value_type = type(value)
assert(
value_type == "string" or value_type == "number",
"arg #3 `value` for function `store_response_header` must be a string or a number"
)
_validate_key(key, 2, "store_response_header")
_validate_value(value, 3, "store_response_header")

local rl_ctx = _get_or_create_rl_ctx(ngx_ctx)
rl_ctx[key] = value
end


function _M.get_stored_response_header(ngx_ctx, key)
assert(
type(key) == "string",
"arg #2 `key` for function `get_stored_response_header` must be a string"
)
_validate_key(key, 2, "get_stored_response_header")

if not _has_rl_ctx(ngx_ctx) then
return nil
end

if not _has_rl_ctx(ngx_ctx) then
return nil
Expand Down
50 changes: 43 additions & 7 deletions t/01-pdk/16-rl-ctx.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,49 +140,85 @@ X-2: 2
location = /t {
rewrite_by_lua_block {
local pdk_rl = require("kong.pdk.private.rate_limiting")
local ok, err
local ok, err, errmsg
ok, err = pcall(pdk_rl.store_response_header, ngx.ctx, nil, 1)
assert(not ok, "pcall should fail")
errmsg = string.format(
"arg #%d `key` for function `%s` must be a string, got %s",
2,
"store_response_header",
type(nil)
)
assert(
err:find("arg #2 `key` for function `store_response_header` must be a string", nil, true),
err:find(errmsg, nil, true),
"unexpected error message: " .. err
)
for _k, v in ipairs({ 1, true, {}, function() end, ngx.null }) do
ok, err = pcall(pdk_rl.store_response_header, ngx.ctx, v, 1)
assert(not ok, "pcall should fail")
errmsg = string.format(
"arg #%d `key` for function `%s` must be a string, got %s",
2,
"store_response_header",
type(v)
)
assert(
err:find("arg #2 `key` for function `store_response_header` must be a string", nil, true),
err:find(errmsg, nil, true),
"unexpected error message: " .. err
)
end
ok, err = pcall(pdk_rl.store_response_header, ngx.ctx, "X-1", nil)
assert(not ok, "pcall should fail")
errmsg = string.format(
"arg #%d `value` for function `%s` must be a string or a number, got %s",
3,
"store_response_header",
type(nil)
)
assert(
err:find("arg #3 `value` for function `store_response_header` must be a string or a number", nil, true),
err:find(errmsg, nil, true),
"unexpected error message: " .. err
)
for _k, v in ipairs({ true, {}, function() end, ngx.null }) do
ok, err = pcall(pdk_rl.store_response_header, ngx.ctx, "X-1", v)
assert(not ok, "pcall should fail")
errmsg = string.format(
"arg #%d `value` for function `%s` must be a string or a number, got %s",
3,
"store_response_header",
type(v)
)
assert(
err:find("arg #3 `value` for function `store_response_header` must be a string or a number", nil, true),
err:find(errmsg, nil, true),
"unexpected error message: " .. err
)
end
ok, err = pcall(pdk_rl.get_stored_response_header, ngx.ctx, nil)
assert(not ok, "pcall should fail")
errmsg = string.format(
"arg #%d `key` for function `%s` must be a string, got %s",
2,
"get_stored_response_header",
type(nil)
)
assert(
err:find("arg #2 `key` for function `get_stored_response_header` must be a string", nil, true),
err:find(errmsg, nil, true),
"unexpected error message: " .. err
)
for _k, v in ipairs({ 1, true, {}, function() end, ngx.null }) do
ok, err = pcall(pdk_rl.get_stored_response_header, ngx.ctx, v)
assert(not ok, "pcall should fail")
errmsg = string.format(
"arg #%d `key` for function `%s` must be a string, got %s",
2,
"get_stored_response_header",
type(v)
)
assert(
err:find("arg #2 `key` for function `get_stored_response_header` must be a string", nil, true),
err:find(errmsg, nil, true),
"unexpected error message: " .. err
)
end
Expand Down

1 comment on commit 65201e8

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bazel Build

Docker image available kong/kong:65201e81fe44807b5abf76b9ef91caeaef0727ba
Artifacts available https://github.com/Kong/kong/actions/runs/9574378202

Please sign in to comment.