fix(sentry): bound endContext flush timeout and guarantee context release - #1084
fix(sentry): bound endContext flush timeout and guarantee context release#1084huangdijia wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 59 minutes Limit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b023e3fddc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Resolve the effective flush timeout here so that callers that omit it | ||
| // (e.g. SentrySdk::endContext()) can never block indefinitely; see DEFAULT_FLUSH_TIMEOUT. | ||
| $this->removeContextById($runtimeContextId, $timeout ?? self::DEFAULT_FLUSH_TIMEOUT); |
There was a problem hiding this comment.
When CoHttpTransport's channel is full, this timeout does not prevent endContext() from hanging: the logs and metrics flushes run first and eventually call CoHttpTransport::send(), whose channel push uses the separate $this->timeout at src/sentry/src/Transport/CoHttpTransport.php:73. The new 0 only reaches $client->flush() afterward, and this component's CoHttpTransport::close() at lines 78-81 ignores that argument entirely. Consequently the deferred SentrySdk::endContext() calls can still block in the exact full-channel scenario this change targets; the enqueue timeout itself must be bounded or the aggregator flushes must avoid blocking.
Useful? React with 👍 / 👎.
1 participant
问题
endContext 同步 flush 在通道满时阻塞协程结束
修改
flush 超时贯通,null 默认 0;先释放 context 后尽力 flush
测试
vendor/bin/pest --group=sentry 全通过