Fix serve trace empty match to return 200 with empty JSON result - #127
Open
gyp15618976002 wants to merge 1 commit into
Open
gyp15618976002 wants to merge 1 commit into
gyp15618976002 wants to merge 1 commit into
Conversation
c2j
reviewed
Aug 3, 2026
c2j
left a comment
Owner
There was a problem hiding this comment.
⚠️ 建议在 PR #128 合并后 rebase
验证结果
| 检查项 | 结果 |
|---|---|
cargo check --features serve |
✅ |
| 集成测试 serve_api | ✅ 8/8 |
代码评审
trace 空匹配返回 200: ✅
- 将"查询无结果"视为正常业务状态(200)而非资源不存在(404),与 search-sql 端点行为一致
- 返回结构与正常 trace 响应完全一致(target=null, callers=[], callees=[], caller_count=0, callee_count=0, truncated=false)
⚠️ 需要关注
-
路由顺序: 此 PR 基于旧的主分支,
.fallback()仍在.layer(middleware)之后,导致 404 请求绕过 access log。PR #128 已修复此问题。建议在 #128 合并后 rebase。 -
测试端口冲突:
test_serve_trace_empty_match_returns_200使用 port 19883,与 #128 的test_serve_access_log_combined_format相同。rebase 后需改为不同端口(如 19884)。
建议操作
# 在 #128 合并后:
git fetch origin main
git rebase origin/main
# 修改测试端口为 19884
cargo test --features serve --test serve_api -- --test-threads=1
c2j
requested changes
Aug 10, 2026
c2j
left a comment
Owner
There was a problem hiding this comment.
PR #127 Review — Fix serve trace empty match to return 200 with empty JSON result
总结:/api/v1/trace 在无匹配时从 404 改为 200 并返回空结构体;测试已新增验证此行为。
需要作者处理:
- 请在 PR 描述或 CHANGELOG 明确为何将 404 改为 200(兼容性/语义原因),并列出受影响的已知客户端场景。
- handler 签名由 Result<impl IntoResponse, StatusCode> 改为 Json,请确认路由注册与中间件兼容;若项目其他 handler 仍使用 Result,建议保持一致或在 PR 描述说明原因。
- 建议再增加一个测试,确保在出现内部错误时仍返回合适的错误状态码,而不是统一映射为 200。
- 当前 PR 状态为 CONFLICTING,请先 rebase/resolve 与 main 的冲突并重新 push。
- 在 PR 里列出本地复现命令输出(cargo build / cargo test --features serve)的快照,便于审查者核对。
结论建议:在完成上述项后再合并。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
125问题修复