Conversation
审查者指南本 PR 为 V1 和 V105 日志上传增加缓存链接的远端存活校验:通过派生的 /load_data 查询接口确认链接状态,在失效或无法确认时重新上传,并在不确定且重传失败时安全地带提示回退到旧链接,同时补充了覆盖关键状态和请求构造的测试。 缓存日志 URL 校验与重新上传时序图sequenceDiagram
participant U as 日志请求方
participant L as uploadV1_or_uploadV105
participant DB as 本地数据库
participant B as 日志后端
U->>L: 上传(env)
L->>DB: LogGetUploadInfo()
DB-->>L: 缓存的 URL 和时间戳
alt 缓存 URL 比日志更新
L->>B: 使用 key、password、Range 和 Bearer token GET 派生的 /load_data
alt 2xx 响应
B-->>L: 日志存在
L-->>U: 返回缓存 URL
else 404 或 410
B-->>L: 日志缺失
L->>B: 上传日志
alt 上传成功
B-->>L: 新 URL
L-->>U: 返回新 URL
else 上传失败
L-->>U: 返回上传错误,不返回过期 URL
end
else 网络错误、超时或 5xx
B-->>L: 状态未知
L->>B: 上传日志
alt 上传成功
B-->>L: 新 URL
L-->>U: 返回新 URL
else 上传失败
L-->>U: 返回带警告的缓存 URL
end
end
else 日志已更改或没有缓存 URL
L->>B: 上传日志
B-->>L: 新 URL 或上传失败
L-->>U: 返回上传结果
end
缓存日志 URL 探测决策流程图flowchart TD
A[缓存的上传 URL 比日志更新] --> B[提取 key 和 password]
B --> C[从后端 /log 推导 /load_data]
C --> D[使用 3 秒超时进行探测]
D --> E{探测结果}
E -->|2xx| F[复用缓存 URL]
E -->|404 或 410| G[重新上传日志]
E -->|网络错误、超时或 5xx| G
G --> H{上传成功}
H -->|是| I[保存并返回新 URL]
H -->|否且状态明确缺失| J[返回上传失败]
H -->|否且状态未知| K[返回带警告的缓存 URL]
文件级变更
针对关联 Issue 的评估
提示和命令与 Sourcery 交互
自定义使用体验访问你的控制面板以:
获取帮助Original review guide in EnglishReviewer's Guide本 PR 为 V1 和 V105 日志上传增加缓存链接的远端存活校验:通过派生的 /load_data 查询接口确认链接状态,在失效或无法确认时重新上传,并在不确定且重传失败时安全地带提示回退到旧链接,同时补充了覆盖关键状态和请求构造的测试。 Sequence diagram for cached log URL validation and re-uploadsequenceDiagram
participant U as Log requester
participant L as uploadV1_or_uploadV105
participant DB as Local database
participant B as Log backend
U->>L: Upload(env)
L->>DB: LogGetUploadInfo()
DB-->>L: cached URL and timestamps
alt Cached URL is newer than log
L->>B: GET derived /load_data with key, password, Range, Bearer token
alt 2xx response
B-->>L: Log exists
L-->>U: Return cached URL
else 404 or 410
B-->>L: Log missing
L->>B: Upload log
alt Upload succeeds
B-->>L: New URL
L-->>U: Return new URL
else Upload fails
L-->>U: Return upload error without stale URL
end
else Network error, timeout, or 5xx
B-->>L: Unknown status
L->>B: Upload log
alt Upload succeeds
B-->>L: New URL
L-->>U: Return new URL
else Upload fails
L-->>U: Return cached URL with warning
end
end
else Log changed or no cached URL
L->>B: Upload log
B-->>L: New URL or upload failure
L-->>U: Return upload result
end
Flow diagram for cached log URL probe decisionflowchart TD
A[Cached upload URL is newer than log] --> B[Extract key and password]
B --> C[Derive /load_data from backend /log]
C --> D[Probe with 3-second timeout]
D --> E{Probe result}
E -->|2xx| F[Reuse cached URL]
E -->|404 or 410| G[Re-upload log]
E -->|Network error timeout or 5xx| G
G --> H{Upload succeeds}
H -->|Yes| I[Save and return new URL]
H -->|No and status missing| J[Return upload failure]
H -->|No and status unknown| K[Return cached URL with warning]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
嘿——我发现了 1 个问题
给 AI Agent 的提示
请处理此次代码审查中的评论:
## 单独评论
### 评论 1
<location path="dice/storylog/upload_v1.go" line_range="79-82" />
<code_context>
env.Log.Errorf("记录Log上传信息失败: %v", errDB)
}
if len(url) == 0 {
+ if fallbackURL, notice, ok := fallbackToCachedLogURL(&env, cachedURL, probeResult); ok {
+ return fallbackURL, notice, nil
+ }
return "", env.Notice, errors.New("上传 log 到服务器失败,未能获取染色器链接")
}
return url, env.Notice, nil
</code_context>
<issue_to_address>
**问题(更广泛的影响):** 当缓存链接探测结果未知,且在 `LogGetAllLines`、`formatAndBackup`/`formatAndBackupV105` 或压缩/文件生成过程中为重新上传进行本地准备失败时,函数会在到达 `fallbackToCachedLogURL` 之前返回该错误;因此,它会丢弃可能仍然有效的缓存 URL,而不是带着不确定性提示返回该 URL。
**触发条件:** 当无法确认缓存 URL,且重新上传在调用后端上传接口之前失败时。
**建议修复:** 对探测结果未知时的所有重新上传失败,都通过 `fallbackToCachedLogURL` 处理;或者使用覆盖完整重新上传流程的延迟回退机制。
</issue_to_address>Sourcery 评估
需要人工审查。 首先需要处理 1 个发现;当缓存 URL 的探测结果无法确定时,此更改会再次上传日志,并可能创建第二条存储在外部的日志记录;即使回滚此更改,也不会删除这次上传,不过其影响范围有限且可以修复。它还可能返回一个已经失效的缓存链接,这属于普通的运行时失败,而不是不可逆的数据或访问权限变更。
阻塞性发现:dice/storylog/upload_v1.go:82
帮助我变得更有用!请在每条评论上点击 👍 或 👎,我会利用反馈来改进审查结果。
Original comment in English
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="dice/storylog/upload_v1.go" line_range="79-82" />
<code_context>
env.Log.Errorf("记录Log上传信息失败: %v", errDB)
}
if len(url) == 0 {
+ if fallbackURL, notice, ok := fallbackToCachedLogURL(&env, cachedURL, probeResult); ok {
+ return fallbackURL, notice, nil
+ }
return "", env.Notice, errors.New("上传 log 到服务器失败,未能获取染色器链接")
}
return url, env.Notice, nil
</code_context>
<issue_to_address>
**issue (broader_impact):** When the cached-link probe is unknown and local preparation for re-upload fails in `LogGetAllLines`, `formatAndBackup`/`formatAndBackupV105`, or compression/file generation, the function returns that error before reaching `fallbackToCachedLogURL`; it therefore discards the potentially valid cached URL instead of returning it with an uncertainty notice.
**Triggers:** When the cached URL cannot be confirmed and the re-upload fails before the backend upload call.
**Suggested fix:** Route all re-upload failures after an unknown probe through `fallbackToCachedLogURL`, or use a deferred fallback covering the complete re-upload path.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and when probing the cached URL is inconclusive, this change uploads the log again and may create a second externally stored log record; reverting the change would not remove that upload, although the impact is bounded and repairable. It can also return a cached link that may no longer work, which is an ordinary runtime failure rather than an irreversible data or access change.
Blocking findings: dice/storylog/upload_v1.go:82
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if fallbackURL, notice, ok := fallbackToCachedLogURL(&env, cachedURL, probeResult); ok { | ||
| return fallbackURL, notice, nil | ||
| } | ||
| return "", env.Notice, errors.New("上传 log 到服务器失败,未能获取染色器链接") |
There was a problem hiding this comment.
问题(更广泛的影响): 当缓存链接探测结果未知,且在 LogGetAllLines、formatAndBackup/formatAndBackupV105 或压缩/文件生成过程中为重新上传进行本地准备失败时,函数会在到达 fallbackToCachedLogURL 之前返回该错误;因此,它会丢弃可能仍然有效的缓存 URL,而不是带着不确定性提示返回该 URL。
触发条件: 当无法确认缓存 URL,且重新上传在调用后端上传接口之前失败时。
建议修复: 对探测结果未知时的所有重新上传失败,都通过 fallbackToCachedLogURL 处理;或者使用覆盖完整重新上传流程的延迟回退机制。
Original comment in English
issue (broader_impact): When the cached-link probe is unknown and local preparation for re-upload fails in LogGetAllLines, formatAndBackup/formatAndBackupV105, or compression/file generation, the function returns that error before reaching fallbackToCachedLogURL; it therefore discards the potentially valid cached URL instead of returning it with an uncertainty notice.
Triggers: When the cached URL cannot be confirmed and the re-upload fails before the backend upload call.
Suggested fix: Route all re-upload failures after an unknown probe through fallbackToCachedLogURL, or use a deferred fallback covering the complete re-upload path.
|
O 这个是用于第三方log站点的?官方的目前似乎还没有失效过 |
是的,要求自建API永久化存储不现实。 |
背景
SealDice 会在日志内容没有变化时,直接复用本地数据库中保存的
upload_url,不再请求日志后端。当日志后端启用滚动清理后,远端日志数据可能已经被删除,但 SealDice 本地仍保留原链接。此时用户执行
.log get或从 UI 提取日志,仍会得到已经失效的链接。Fixes #1563
修改内容
在复用缓存日志链接前增加远端存活校验:
key和密码。/log替换为/load_data,不写死路径前缀:/dice/api/log→/dice/api/load_data/custom/path/log→/custom/path/load_data2xx:远端日志仍存在,继续复用缓存链接。404/410:远端日志已明确失效,重新上传并保存新链接。5xx:无法确认链接状态,尝试重新上传。该逻辑同时应用于 V1 和 V105 日志格式,因此
.log get与 UI 日志提取会保持一致。改动范围
Sourcery 总结
确保在重复使用缓存的日志链接前对其进行验证,并在远程日志已过期时安全恢复。
错误修复:
增强功能:
测试:
Original summary in English
Sourcery 总结
在复用缓存日志链接前确认其仍然有效,并在失效时安全恢复日志上传。
Bug 修复:
功能增强:
测试:
Original summary in English
Sourcery 摘要
在复用缓存日志链接前验证其有效性,并在失效或无法确认时安全地恢复日志上传。
错误修复:
改进:
测试:
Original summary in English
Summary by Sourcery
在复用缓存日志链接前验证其有效性,并在失效或无法确认时安全地恢复日志上传。
Bug Fixes:
Enhancements:
Tests:
Original summary in English