Skip to content

WorkflowContext add metadata onRunFailed #69#85

Merged
Jiakaic merged 1 commit intoLianjiaTech:developfrom
codingDuan:fix_metadata_enhancement
Nov 11, 2025
Merged

WorkflowContext add metadata onRunFailed #69#85
Jiakaic merged 1 commit intoLianjiaTech:developfrom
codingDuan:fix_metadata_enhancement

Conversation

@codingDuan
Copy link
Copy Markdown

@codingDuan codingDuan commented Nov 11, 2025

Pull Request

Description

add metadata onWorkflowRunFailed

Fixes # (issue)

Type of Change

Please delete options that are not relevant.

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🧹 Code refactoring (no functional changes)
  • ⚡ Performance improvement
  • 🔧 Configuration change

Component

Please check the components affected by this PR:

  • 🔧 API (Backend)
  • 🎨 Web (Frontend)
  • 🐳 Docker/Infrastructure
  • 📖 Documentation
  • 🧪 Tests
  • 📦 Dependencies

Testing

Please describe the tests that you ran to verify your changes:

  • Unit tests pass (npm test / mvn test)
  • Integration tests pass
  • Manual testing completed
  • Docker build successful

Test Configuration:

  • OS: [e.g. Ubuntu 20.04]
  • Node.js version: [e.g. 20.x]
  • Java version: [e.g. 8]

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Screenshots (if applicable)

Please add screenshots to help explain your changes.

Additional Notes

Add any additional notes, concerns, or questions here.

@Jiakaic
Copy link
Copy Markdown
Member

Jiakaic commented Nov 11, 2025

@claude 检查提交的代码,使用中文回复

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 11, 2025

Claude finished @Jiakaic's task —— View job


代码审查完成 ✅

任务列表

  • 阅读仓库配置文件 (CLAUDE.md)
  • 审查 WorkflowBatchRunCallback.java 的修改
  • 审查 WorkflowRunBlockingCallback.java 的修改
  • 审查 WorkflowRunNotifyCallback.java 的修改
  • 审查 WorkflowRunStreamingCallback.java 的修改
  • 分析代码变更的一致性和正确性
  • 提供详细的审查反馈

审查结果

🎯 变更概述

此PR在4个工作流回调类的 onWorkflowRunFailed 方法中统一添加了 responseWorkflowMetaData(context, data) 调用,确保工作流失败时也能返回元数据信息。

✅ 积极方面

  1. 一致性很好: 所有4个回调类都进行了相同的修改,保持了API的一致性
  2. 逻辑合理: 在 onWorkflowRunSucceeded 方法中已经调用了 responseWorkflowMetaData,现在失败场景也包含元数据是合理的
  3. 代码位置正确: 都在synchronized块内进行修改,保证了线程安全
  4. 方法调用顺序一致: 都是在 responseWorkflowInforesponseWorkflowError 之后调用,与成功场景的顺序保持一致

📋 代码分析

修改的文件和位置:

  • WorkflowBatchRunCallback.java:42 - 批量运行回调
  • WorkflowRunBlockingCallback.java:55 - 阻塞运行回调
  • WorkflowRunNotifyCallback.java:39 - 通知回调
  • WorkflowRunStreamingCallback.java:70 - 流式回调

调用的方法:

  • responseWorkflowMetaData(context, data) 来自父类 WorkflowCallbackAdaptor.java:19-21
  • 该方法将 context.getMetadata() 放入响应数据的 "metadata" 字段

🔍 潜在考虑点

  1. 元数据大小: 如果 context.getMetadata() 包含大量数据,在失败场景下也返回可能会增加响应体积,但这通常是可接受的,因为调试时元数据很有用

  2. 错误场景的元数据: 失败时的元数据可能对排查问题很有价值,这是个好的设计决策

💡 建议

  1. 测试覆盖: 建议补充单元测试,验证失败场景下元数据是否正确返回
  2. 文档更新: 如果有API文档,建议更新说明失败响应现在也包含metadata字段

🎯 总体评价

这是一个质量很高的小型修复,解决了API一致性问题。代码变更简洁、一致、安全,符合现有的代码模式。修改使得工作流失败时的响应信息更完整,有助于问题排查和调试。

推荐合并


Job run

@Jiakaic Jiakaic merged commit b1879e7 into LianjiaTech:develop Nov 11, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants