fix: 修正 MXU_WEBHOOK_ACTION 的 HTTP 請求處理#259
Conversation
修正 MXU webhook HTTP 請求處理
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并且留下了一些高层次的反馈:
- 当在 webhook 配置中同时提供
body和json时,body.or(json)会在没有提示的情况下优先选择body;建议显式地校验并拒绝这种互相冲突的配置,以避免产生令人意外的行为。 - 超时限制范围
clamp(1, 300)有点像“魔法数字”;将这些限制提取成具名常量会让行为更清晰,也更方便后续调整。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- 当在 webhook 配置中同时提供 `body` 和 `json` 时,`body.or(json)` 会在没有提示的情况下优先选择 `body`;建议显式地校验并拒绝这种互相冲突的配置,以避免产生令人意外的行为。
- 超时限制范围 `clamp(1, 300)` 有点像“魔法数字”;将这些限制提取成具名常量会让行为更清晰,也更方便后续调整。
## Individual Comments
### Comment 1
<location path="src-tauri/src/mxu_actions.rs" line_range="420-429" />
<code_context>
};
- info!("[MXU_WEBHOOK] Sending GET request to: {}", url);
+ let body = body.or(json);
+ let method = match parse_webhook_method(method.as_deref(), body.is_some()) {
+ Ok(value) => value,
</code_context>
<issue_to_address>
**issue (bug_risk):** 使用 `json(&value)` 可能会覆盖用户提供的 `Content-Type` 头。
对于非字符串的请求体,`request.json(&value)` 总是会设置 `Content-Type: application/json`,这会覆盖在 `headers` 中设置的任何头。如果调用方必须控制内容类型,建议通过 `request.body(serde_json::to_vec(&value)?/to_string())` 手动序列化,以保留他们设置的头,或者仅在尚未设置 `Content-Type` 时才使用 `json()`。
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- When both
bodyandjsonare provided in the webhook config,body.or(json)silently prefersbody; consider explicitly validating and rejecting conflicting settings to avoid surprising behavior. - The timeout clamp range
clamp(1, 300)is a bit of a magic number; extracting these limits into named constants would make the behavior clearer and easier to adjust later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When both `body` and `json` are provided in the webhook config, `body.or(json)` silently prefers `body`; consider explicitly validating and rejecting conflicting settings to avoid surprising behavior.
- The timeout clamp range `clamp(1, 300)` is a bit of a magic number; extracting these limits into named constants would make the behavior clearer and easier to adjust later.
## Individual Comments
### Comment 1
<location path="src-tauri/src/mxu_actions.rs" line_range="420-429" />
<code_context>
};
- info!("[MXU_WEBHOOK] Sending GET request to: {}", url);
+ let body = body.or(json);
+ let method = match parse_webhook_method(method.as_deref(), body.is_some()) {
+ Ok(value) => value,
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `json(&value)` may override user-provided `Content-Type` headers.
For non-string bodies, `request.json(&value)` always sets `Content-Type: application/json`, which will override any header set in `headers`. If callers must control the content type, consider serializing manually via `request.body(serde_json::to_vec(&value)?/to_string())` so their header is preserved, or only use `json()` when `Content-Type` is not already present.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| let body = body.or(json); | ||
| let method = match parse_webhook_method(method.as_deref(), body.is_some()) { | ||
| Ok(value) => value, | ||
| Err(e) => { | ||
| warn!("[MXU_WEBHOOK] {}", e); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| let headers = match build_webhook_headers(headers) { |
There was a problem hiding this comment.
issue (bug_risk): 使用 json(&value) 可能会覆盖用户提供的 Content-Type 头。
对于非字符串的请求体,request.json(&value) 总是会设置 Content-Type: application/json,这会覆盖在 headers 中设置的任何头。如果调用方必须控制内容类型,建议通过 request.body(serde_json::to_vec(&value)?/to_string()) 手动序列化,以保留他们设置的头,或者仅在尚未设置 Content-Type 时才使用 json()。
Original comment in English
issue (bug_risk): Using json(&value) may override user-provided Content-Type headers.
For non-string bodies, request.json(&value) always sets Content-Type: application/json, which will override any header set in headers. If callers must control the content type, consider serializing manually via request.body(serde_json::to_vec(&value)?/to_string()) so their header is preserved, or only use json() when Content-Type is not already present.
摘要
這個 PR 修正
MXU_WEBHOOK_ACTION目前只能送出 GET 請求、且非成功 HTTP 狀態仍被視為成功的問題。原本 webhook action 只支援:
{ "url": "https://example.com/webhook" }這對需要 POST JSON payload 的 webhook 服務不夠完整,例如 Discord Webhook。
主要變更
保留舊版
{ "url": "..." }格式,預設仍以 GET 請求送出。新增支援:
methodheadersbodyjsontimeout_secsfail_on_non_success當設定中包含
body或json,且沒有明確指定method時,預設改用 POST。HTTP 回應狀態不是 2xx 時,預設回傳
false,避免 4xx / 5xx 被誤判為 webhook 執行成功。log 不再輸出完整 webhook URL,避免 webhook token 被寫入 log。
使用範例
{ "url": "https://discord.com/api/webhooks/xxx/yyy", "method": "POST", "headers": { "Content-Type": "application/json" }, "body": { "content": "MXU 任務已完成" } }相容性
舊設定格式仍可正常使用,不需要修改既有的
interface.json設定。驗證
已執行:
cd src-tauri cargo check結果通過。
Summary by Sourcery
扩展 MXU webhook 自定义动作,以支持可配置的 HTTP 请求和更严格的成功判定标准。
新功能:
缺陷修复:
增强改进:
Original summary in English
Summary by Sourcery
Extend MXU webhook custom action to support configurable HTTP requests and stricter success criteria.
New Features:
Bug Fixes:
Enhancements: