Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bk-error-wrapper): response from plugin been wrapped #82

Merged

Conversation

wklken
Copy link
Collaborator

@wklken wklken commented Oct 10, 2024

Description

which is not expected

原来:

  1. apisix/openresty + upstream error + 插件: 返回的 status>=400的,全部封装网关的返回结果
  2. 蓝鲸插件的插件返回都会调用 errorx.exit_with_apigw_err, 有设置 ctx.var.bk_apigw_error

问题:
插件返回的code+response body会被wrap

决策:

  1. upstream error还是按原来的逻辑处理
  2. apisix/openresty + 插件 返回的不再判断
  3. errorx.exit_with_apigw_err 保持跟原来逻辑一样

TODO:

  • 修改单测
  • 确认所有蓝鲸插件响应都是 errorx.exit_with_apigw_err

手工测试:

  1. 插件返回502/500 + body => 不再封装 ok
  2. 上游返回502/500 => 跟原来一样 ok
  3. dns解析失败 502 => 通过 error_page 502 封装成 json,跟原来一样 ok
  4. apisix 内部返回 500 => 通过 error_page 500 封装成 json, 跟原来一样 ok
  5. 504 timeout超时 => 跟原来一样 ok

Fixes # (issue)

Checklist

  • 填写 PR 描述及相关 issue (write PR description and related issue)
  • 代码风格检查通过 (code style check passed)
  • PR 中包含单元测试 (include unit test)
  • 单元测试通过 (unit test passed)
  • 本地开发联调环境验证通过 (local development environment verification passed)

@wklken wklken marked this pull request as draft October 10, 2024 12:28
@wklken wklken marked this pull request as ready for review October 11, 2024 03:58
Copy link
Member

@Han-Ya-Jun Han-Ya-Jun left a comment

Choose a reason for hiding this comment

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

LGTM

@wklken wklken merged commit 86428cc into TencentBlueKing:master Oct 12, 2024
3 checks passed
@wklken wklken deleted the fix_plugin_response_been_wrapped_master branch October 12, 2024 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants