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(streaming): resolve ctx diverge in server-side streaming #1471

Merged
merged 2 commits into from
Aug 28, 2024

Conversation

DMwangnima
Copy link
Contributor

@DMwangnima DMwangnima commented Aug 1, 2024

What type of PR is this?

fix

Check the PR title.

  • This PR title match the format: <type>(optional scope): <description>
  • The description of this PR title is user-oriented and clear enough for others to understand.
  • Attach the PR updating the user documentation if the current PR requires user awareness at the usage level. User docs repo

(Optional) Translate the PR title into Chinese.

fix: 修复 server 侧 streaming 接口 ctx 分叉的问题

(Optional) More detailed description for this PR(en: English/zh: Chinese).

en:
Currently in the server-side streaming scenario, the middleware func(ctx context.Context, request, response interface{}) after processing the ctx (e.g. WithValue) is not perceived by the Stream.Context() in the handler.
Use contextStream to wrap the Stream provided to the user handler, inject the middleware-processed ctx, and override the Stream.Context() method exposed to the user to solve this problem.
zh(optional):
当前在 server 侧 streaming 场景,middleware func(ctx context.Context, request, response interface{}) 对 ctx 进行处理后(例如 WithValue)并不能通过 handler 中的 Stream.Context() 感知到。
利用 contextStream wrap 一下提供给用户 handler 的 Stream,注入经过 middleware 处理后的 ctx,并重写暴露给用户的 Stream.Context() 方法来解决这个问题。

(Optional) Which issue(s) this PR fixes:

(optional) The PR that updates user documentation:

@DMwangnima DMwangnima requested review from a team as code owners August 1, 2024 07:59
@DMwangnima DMwangnima changed the base branch from develop to online/fix_ctx_diverge August 12, 2024 08:07
@DMwangnima DMwangnima changed the base branch from online/fix_ctx_diverge to develop August 12, 2024 08:08
joway
joway previously approved these changes Aug 12, 2024
@DMwangnima DMwangnima changed the title fix: resolve ctx diverge in server-side streaming fix(streaming): resolve ctx diverge in server-side streaming Aug 13, 2024
server/stream.go Outdated Show resolved Hide resolved
server/stream_test.go Outdated Show resolved Hide resolved
YangruiEmma
YangruiEmma previously approved these changes Aug 21, 2024
YangruiEmma
YangruiEmma previously approved these changes Aug 21, 2024
@DMwangnima DMwangnima force-pushed the fix/stream_ctx_diverge branch 2 times, most recently from eabe30d to 854c558 Compare August 28, 2024 08:56
@DMwangnima DMwangnima merged commit 633f72c into cloudwego:develop Aug 28, 2024
39 checks passed
@DMwangnima DMwangnima deleted the fix/stream_ctx_diverge branch August 28, 2024 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants