fix(task): sync Data JSON status when sweepTimedOutTasks marks task as failed (#5715)#5769
Open
zhangzhichaolove wants to merge 2 commits into
Open
fix(task): sync Data JSON status when sweepTimedOutTasks marks task as failed (#5715)#5769zhangzhichaolove wants to merge 2 commits into
zhangzhichaolove wants to merge 2 commits into
Conversation
…s failed (QuantumNous#5715) When sweepTimedOutTasks force-marks a task as FAILURE after timeout, the outer task.Status is updated but task.Data (upstream raw JSON) retains its pre-timeout status (e.g. IN_PROGRESS, QUEUED). Downstream consumers reading data.status see a contradictory state. Add patchDataStatus helper to overwrite the status field inside Data JSON before persisting, ensuring internal and external status are consistent.
Contributor
WalkthroughTimed-out tasks now update both the task record and the embedded JSON payload so the stored status matches the forced failure state. ChangesTimed-out task status sync
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@service/task_polling.go`:
- Line 72: The timeout handling in task polling only forces the top-level task
status to FAILURE, leaving provider-specific status fields stale for some
adapters. Update the task status patching in the polling flow so it also
overwrites the provider-derived status data used by adaptors like
ali/adaptor.go’s status serialization, or adjust those adaptors to prefer
task.Status whenever the row is terminal. Use the existing task.Data mutation
path in task_polling.go and the provider response status fields referenced by
the adaptors to keep client-visible status consistent.
- Around line 636-645: The patchDataStatus logic currently unmarshals task.Data
into a generic map and re-marshals the whole payload, which can change unrelated
numeric fields; update patchDataStatus to modify only the top-level status field
directly from the raw JSON instead of decoding into map[string]any. Keep the
existing behavior around detecting whether status exists, but implement the
replacement in a way that preserves the rest of the original bytes and avoids
touching other fields in task.Data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 32f820ea-fb04-4966-bc70-628c98d36590
📒 Files selected for processing (1)
service/task_polling.go
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
1 participant
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.
Important
📝 变更描述 / Description
任务超时被
sweepTimedOutTasks强制标记为失败时，仅更新了外层task.Status = FAILURE，但task.Data（存储上游原始 JSON 响应）中的status字段未同步修改，导致下游通过TaskModel2Dto查询任务时，外层状态为 FAILURE，而data.status仍为 IN_PROGRESS 或 QUEUED，产生矛盾。修复方式：新增
patchDataStatus辅助函数，在标记超时失败、写入数据库前，解析task.DataJSON 并将其中顶层status字段覆写为FAILURE。使用map[string]json.RawMessage保证其他字段保持原始字节不变，避免 float64 精度丢失。若 Data 为空、非 JSON 对象或不含 status 字段，则原样返回，不破坏现有数据。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。go build ./...），无新增编译错误。📸 运行证明 / Proof of Work
编译验证：
Summary by CodeRabbit