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

bugfix: fix the key parameter problem of sofa-rpc setAttachment() method #5745

Merged
merged 4 commits into from Aug 7, 2023

Conversation

dmego
Copy link
Contributor

@dmego dmego commented Jul 24, 2023

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

sofa-rpc 中,RpcInternalContext.getContext().setAttachment() 方法的 key 参数要求以 . 或者_开头,否则会抛异常

Ⅱ. Does this pull request fix one issue?

fix #3835

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

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

为什么会报错,测试用例测试的时候好像没报错呀,是新的规范吗?

@dmego
Copy link
Contributor Author

dmego commented Jul 24, 2023

@a364176773 我看sofa-rpc源码里是一直都有这个规范,但是在seata中,正常情况下是不会触发这个bug的。因为从代码上看 seata 支持在 sofa-rpc 下做 xid 的透传,是通过 SofaRequest.requestProps 属性来传递实现的,这里的 RpcInternalContext.getContext().setAttachment() 代码一般不会被执行到,除非 Provider 端的 SOFA-RPC 线程里 xid 没有清除,才会进入这里。

/**
* The constant HIDDEN_KEY_BRANCH_TYPE
*/
public static final String HIDDEN_KEY_BRANCH_TYPE = Constants.HIDE_KEY_PREFIX_CHAR + KEY_BRANCH_TYPE;
Copy link
Member

Choose a reason for hiding this comment

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

add comment.

RpcInternalContext.getContext().setAttachment(RootContext.KEY_XID, xid);
RpcInternalContext.getContext().setAttachment(RootContext.KEY_BRANCH_TYPE, branchType.name());
RpcInternalContext.getContext().setAttachment(RootContext.HIDDEN_KEY_XID, xid);
RpcInternalContext.getContext().setAttachment(RootContext.HIDDEN_KEY_BRANCH_TYPE, branchType.name());
Copy link
Member

Choose a reason for hiding this comment

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

Here are some case to prove RpcInternalContext.getContext().setAttachment(RootContext.KEY_XID, xid); not as expected ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test case added

Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

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

LGTM 补充下changelog

@dmego
Copy link
Contributor Author

dmego commented Aug 7, 2023

LGTM 补充下changelog LGTM 补充下changelog

@a364176773 已补充

@slievrly slievrly merged commit 27e9596 into apache:2.x Aug 7, 2023
7 checks passed
@slievrly slievrly added this to the 2.0.0 milestone Aug 7, 2023
@funky-eyes
Copy link
Contributor

develop 也需要同步一个pr

@funky-eyes funky-eyes added type: bug Category issues or prs related to bug. module/integration integration module labels Aug 8, 2023
@dmego
Copy link
Contributor Author

dmego commented Aug 8, 2023

develop 也需要同步一个pr

OK,我同步一下

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/integration integration module type: bug Category issues or prs related to bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sofarpc 组件 sofaRequest.addRequestProp 和 RpcInternalContext.getContext().setAttachment 的key 报错
3 participants