refactor(sentry): simplify transaction null checks using nullsafe operator #942
Conversation
…rator Replace explicit null checks with PHP 8's nullsafe operator (?->) for cleaner and more concise transaction validation. This change maintains the same logic while improving code readability by combining null and method call checks in a single expression. Changes: - CoroutineAspect: Simplified transaction check in process method - EventHandleListener: Updated 5 methods to use nullsafe operator for transaction validation
Walkthrough在协程切面与事件监听器中，将对事务是否存在与是否采样的双重判断，统一替换为使用 PHP 空安全操作符的单一判断：由 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant E as EventHandler
participant C as CurrentHub
participant T as Transaction
Note over E: 处理事件时
E->>C: getCurrentTransaction()
C-->>E: transaction 或 null
alt 未采样或无事务
E->>E: if (!transaction?->getSampled()) return
Note over E: 直接返回，跳过追踪
else 已采样事务
E->>E: 继续处理/记录span等
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.28)At least one path must be specified to analyse. 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 |
…rator (#942) Replace explicit null checks with PHP 8's nullsafe operator (?->) for cleaner and more concise transaction validation. This change maintains the same logic while improving code readability by combining null and method call checks in a single expression. Changes: - CoroutineAspect: Simplified transaction check in process method - EventHandleListener: Updated 5 methods to use nullsafe operator for transaction validation Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
64-66: 使用 nullsafe 运算符合并判空与采样检查，语义等价且更简洁。
if (! $transaction?->getSampled())在 $transaction 为 null 或getSampled()为 false/null 时均为真，等价于原先的双条件早退逻辑。- 建议将上一行注释从“没有活动事务则跳过”更新为“没有活动事务或未采样则跳过”，以与逻辑完全一致。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php(1 hunks)src/sentry/src/Tracing/Listener/EventHandleListener.php(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/*/src/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use the namespace pattern FriendsOfHyperf{ComponentName} in all component PHP source files
Files:
src/sentry/src/Tracing/Aspect/CoroutineAspect.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.php
{src,tests}/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to PSR-12 coding standards across PHP code
Files:
src/sentry/src/Tracing/Aspect/CoroutineAspect.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.php
src/*/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
New components must follow the standard component structure under src/{component-name}/ including .gitattributes, .github, LICENSE, README.md, composer.json, and a src/ subdirectory
Files:
src/sentry/src/Tracing/Aspect/CoroutineAspect.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
| if (! $transaction?->getSampled()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
统一使用 $transaction?->getSampled() 判空+采样检查，保持原有早退行为。
- 改动保持功能不变，避免在 $transaction 为空时调用方法的风险，符合代码库现有风格（PSR-12 间距也保持一致）。
- 请确认组件（及 monorepo 根）composer 最低 PHP 版本约束≥8.0，以确保 nullsafe 语法在分发环境无兼容风险。
运行以下脚本快速检查仓库中各 composer.json 的 PHP 版本约束：
Also applies to: 520-522, 588-590, 656-658, 707-709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Composer PHP 版本约束扫描："
fd -a composer.json | while read -r f; do
req=$(jq -r '.require.php // empty' "$f" 2>/dev/null || true)
if [ -z "$req" ]; then
req=$(rg -nP '"php"\s*:\s*"[^"]+"' "$f" -N -S -o | sed -E 's/.*"php"\s*:\s*"([^"]+)".*/\1/' || true)
fi
printf "%s => %s\n" "$f" "${req:-<未声明>}"
doneLength of output: 3161
在 src/sentry/composer.json 中添加 PHP 最低版本约束
- 根据 monorepo 根 composer.json (
>=8.1)，请在src/sentry/composer.json的require段中增加"php": ">=8.1"以确保 nullsafe 操作符（PHP 8.0+）在分发环境可用。
🤖 Prompt for AI Agents
In src/sentry/src/Tracing/Listener/EventHandleListener.php around lines 407 to
409, the code uses the nullsafe operator but src/sentry/composer.json lacks a
PHP version constraint; open src/sentry/composer.json and add or update the
"require" section to include "php": ">=8.1" (matching the monorepo root), then
run composer validate and composer update (or composer install) to regenerate
lock files so the package declares PHP >=8.1.
Summary
?->)Changes Made
process()methodonBeforeHandle()onAfterHandle()onBeforeWrite()onAfterWrite()onBeforeFlush()Technical Details
Before:
After:
The nullsafe operator automatically handles the null check, making the code more concise and leveraging PHP 8's modern syntax while preserving identical runtime behavior.
Test Plan
Summary by CodeRabbit