fix(drivers/189): support updated time format from 189Cloud - #2918
fix(drivers/189): support updated time format from 189Cloud#2918AmPlace wants to merge 2 commits into
Conversation
Co-Authored-By: Codex <noreply@openai.com>
- Normalize Unicode spaces and preserve existing time layouts. - Add the updated 12-hour layout and focused parser regression tests. Co-Authored-By: Codex <267193182+codex@users.noreply.github.com>
AmPlace
changed the title
|
Hi @AmPlace, Thank you for taking the time to work on this important bug fix! I appreciate your effort in addressing the 189Cloud time format issue. I noticed that PR #2919 by @shelken tackles the same problem with a slightly different approach that handles an additional edge case. Specifically: Key Difference: Timezone HandlingYour current implementation always appends v, err = time.ParseInLocation(f, bs+" +08", time.Local)However, according to issue #2917, the 189Cloud API may now return time strings that already include a timezone, such as: In this case, appending another PR #2919 addresses this by using a two-level loop that tries both the original string and the string with for _, s := range []string{bs, bs + " +08"} {
for _, f := range []string{...} {
v, err = time.ParseInLocation(f, s, time.Local)
if err == nil { break }
}
}This ensures compatibility with both:
Additional Note: Time Format TemplateThe template "Jan 2, 2006 3:04:05 PM -07"SuggestionYou might want to take a look at PR #2919's implementation. If you'd like to update your PR with the timezone handling logic, I'd be happy to see it! Otherwise, both PRs solve the core Unicode space issue excellently. Thanks again for your contribution! 🙌 |
|
Close by #2919 |
|
我还是想把 #2918 这个事问清楚。 先说明白，我不是觉得谁先提 PR 就必须合谁，维护者有权选最终实现。 但我重新看完 #2918 和 #2919 两边的完整时间线以后，我认为这次处理本身就需要一个明确解释。因为两个高度重叠、而且都能解决核心问题的 PR，实际走的根本不是同一条 review 路径。 我最开始的 189pc commit 已经包含了这次实际问题涉及的核心修改：
这里也需要说明：#2918 后来补充 189_tv 的第二个 commit，提交时间晚于 #2919 已经公开覆盖 189pc 和 189_tv 的版本。我看到 #2919 同时处理了 189_tv 后，也把我原本已经完成的 189pc parser 修复和 tests 扩展到了 189_tv。 所以我不主张 #2918 的 189_tv 覆盖早于 #2919。我的主张是：#2918 最早的 189pc commit 在 #2919 创建之前，就已经独立包含了这次核心 parser 修复和 regression tests。 我在 8 月 12 日 13:04 完成了补充 189_tv 的第二个 commit。这个 PC + TV 版本在你开始 review #2919、并首次 所以到这轮 review 开始时，#2918 已经不是最初只修改 189pc 的版本，而是同时包含 189pc、189_tv 和两边 regression tests 的版本。 而且我用真实 189Cloud PC 上传验证过，#2917 里那个 所以 #2918 不是一个没修好、没测试，或者不能解决实际问题的 PR。 这一点你自己在 #2918 下面也明确说了：
所以至少从你自己的评论来看，当时并不存在“#2918 核心修复不成立，只有 #2919 才能修”的情况。 但后面的 review 流程很奇怪。 10:08，#2919 先收到了一份 AI-generated review。它当时的 GitHub review 状态是 更关键的是，10:16，你又对 #2919 提交了一次正式的 而在你首次 后来作者 force-push 了新的 commit，之前的 approval 因此变成 stale 并被 dismissed；作者回复这三个问题以后，#2919 又获得了一次新的 所以从公开记录来看，#2919 实际走的是一条持续推进的：
的路径。 然后 10:21，你才来到 #2918，说 #2919 多处理了一个 timezone edge case，同时跟我说：
正常理解就是，我这个 PR 仍然可以继续改、继续 review，对吧？ 但现在结合完整时间线来看，我收到这句话的时候，#2919 实际上已经获得过一次正式 approval。 如果当时 #2919 已经实际上进入优先合并路径，那么这句“可以继续修改”就很容易让人误以为 #2918 仍然有真实的竞争机会。 所以这里我更想确认： 你 10:21 告诉我可以继续修改 #2918 的时候，#2918 到底还是不是一个实际可能被选择的候选？ 但实际发生的是：
也就是说，从 #2919 作者完成 review 回复，到我的 PR 被关闭只有 55 秒；到 #2919 最终 merge，只有 2 分 15 秒。 所以我现在真正想问的已经不是“为什么只等我 87 分钟”这么简单。 我想问的是：为什么 #2919 在我收到“可以继续修改”的评论之前就已经获得过正式 approval，之后又继续得到明确问题、作者更新和回复、重新 approve、merge，而 #2918 得到的是一句“你可以继续补”，然后在 #2919 作者回复后 55 秒就直接被关闭？ 从 10:21 到关闭 #2918，中间没有 deadline，也没有告诉我 #2919 马上就要 merge，更没有说 87 分钟没回复就算放弃。 无论你 10:21 那句
这个过程。 如果 #2918 当时仍然是一个实际可能被选择的候选，那为什么在 #2919 已经获得过正式 approval 的情况下，不给 #2918 一个正常的异步修改和复审时间，而是在 87 分钟以后直接关闭？ 如果不是，那为什么当时不直接说明已经明确倾向选择 #2919？ 另外，几处技术细节我也一并说明。 先说 timezone。 你当时在 #2918 里说：
但 #2917 的公开错误其实不能证明 raw API 返回值本身已经带 因为旧代码本来就是： v, err = time.ParseInLocation(f, bs+" +08", time.Local)也就是说，parser 收到的是 OpenList 自己构造出来的 我后来用 Go 独立复现过。当 raw 原值为： 也就是不带 timezone，且 AM/PM 前就是 U+202F 空格时，旧代码自己追加 反过来，如果 raw 本来已经带了 而 #2917 公布的错误输入中只有一个 所以 #2917 能证明的是 parser 确实出问题了，但不能靠报错里那个单独的 #2919 支持 raw timestamp 本身已经包含 timezone 的输入，这一点我没有异议。但这只能说明实现覆盖了这种输入形式；它是否是修复 #2917 所必需的，还需要 raw response 等独立证据支持。 #2919 中 而 #2918 这边已经有真实 189Cloud 上传验证，原来的报错确实已经消失。 更关键的是，就算你认为这个 timezone case 很重要，你 10:21 的评论也已经明确告诉我可以把同样的 timezone handling 补到 #2918。这说明这个差异本身就是一个可以通过 review 要求补上的修改项，而不是 #2918 无法继续推进的根本技术问题。 再说说你提到的时间格式问题。 就是： 混用 24 小时字段与 AM/PM 那个点。 这个写法语义上确实不规范，但 Go parser 实际能够接受它。更重要的是，这个 layout 不是 #2918 新引入的，而是共同 base 中已有的 legacy 格式。 #2918 保留了原有接受范围，同时为这次新格式新增了正确的： #2919 把旧 layout 也一起改成了 另外补充一句：最终 #2919 还把 所以如果最后选择 #2919 的主要理由是“它多处理了 raw 自带时区、修正了旧格式模板”，那我更想问： 这两处都不是 #2918 无法解决的根本性技术问题，其中 timezone handling 你当时甚至已经明确邀请我补充。既然如此，为什么实际流程不是给 #2918 一个有效的修改和复审机会，再比较两个 PR，而是在 #2919 作者回复后 55 秒直接关闭 #2918？ 我没有否认 #2919 当时的版本在细节处理上更完整。 但这最多可以解释：为什么你认为当时的 #2919 更完整。 它不能自动解释：为什么 #2918 没有获得把这些细节补齐以后再比较的机会。 这是两个完全不同的问题。 最后我把问题明确成三点，希望你逐条回应：
如果项目最终仍然决定保留 #2919，我希望至少在 #2919、Release Note 或其他公开记录中补充 #2918 / @AmPlace 的 cross-reference，准确说明：#2918 最早的 189pc commit 在 #2919 之前已经独立实现了核心 parser 修复和 regression tests；后续 #2918 又扩展到了 189_tv，并完成了真实环境验证。 我不主张 #2918 的 189_tv 覆盖早于 #2919，也不是在没有代码复用证据的情况下要求追加 现在 #2919 没有 cross-reference #2918，merge 记录里也没有 @AmPlace，#2918 最后只剩一句：
这个结果我不能接受，我希望这次不要再用一句 Close by #2919 就结束讨论，而是正面回答这三个问题。 |
|
你好，感谢您为这个BUG贡献，也提出了疑问：
请注意，不是谁先提PR就会采纳谁的贡献，#2919在合并的时候已经满足修复问题以及我们评审 |
这份回复没有回答我最核心的流程问题。更严重的是，你用来解释选择结果的 AI 对比报告，对 #2918 的实际代码和 tests 存在多处可以直接核对的事实错误，我这篇回答是经过 gpt 润色，包括前面我的回答，但是事实核对还有日志包括代码思路都是我自己提供的的，不是把问题丢给 ai 让他替我判断的，我刚才又核对了 #2918 最早的 commit bac76bb。这个正确的 comma + 12-hour layout、expected hour=15 的 legacy test，以及 invalid negative test，在 #2918 第一版里就已经存在。 所以这里甚至不是“AI 报告拿到了 #2918 的旧版本”这么简单。 这份报告描述的代码并不对应 #2918 的第一版，也不对应后来的版本。 那我现在更想知道，这份被用来解释“为什么择优选择 #2919”的 AI 报告，当时到底是基于什么代码生成的？ 坦白说，如果这份报告确实参与了当时的选择，那现在的问题就不只是“两个方案判断不同”了，而是用于择优的评估材料本身就没有准确读到被比较 PR 的实际代码。 1. AI 报告引用的 #2918 代码和当时实际代码对不上报告声称 #2918 的 layouts 是： "Jan 2, 2006 15:04:05 PM -07",
"Jan 2, 2006 15:04:05 PM -07",
"2006-01-02 15:04:05 -07",也就是重复了两次 legacy layout，并据此得出 #2918 没有正确新增 comma + 12-hour layout 的结论。 但 #2918 的实际代码是： for _, f := range []string{
"2006-01-02 15:04:05 -07",
"Jan 2, 2006 15:04:05 PM -07",
"Jan 2, 2006, 3:04:05 PM -07",
} {
v, err = time.ParseInLocation(f, bs+" +08", time.Local)
}其中： 就是这次为新格式新增的正确 comma + 12-hour layout。 所以 AI 报告展示的 #2918 代码并不存在。它重复了旧 layout，同时漏掉了 #2918 实际新增的关键 layout。 这不是对实现优劣的评价差异，而是对被评审代码的事实引用错误。 2. 报告把 parser input 当成 raw API response，而且与我的真实运行日志矛盾报告反复断言：
并以此得出： 但旧代码本来就是： bs := strings.Trim(string(b), "\"")
for _, f := range []string{
"2006-01-02 15:04:05 -07",
"Jan 2, 2006 15:04:05 PM -07",
} {
v, err = time.ParseInLocation(f, bs+" +08", time.Local)
}parser error 展示的是传给 它不是 raw HTTP response。 我此前已经用旧代码独立复现过：当 旧代码追加 反过来，如果 相应 error 中也会显示两个 而且现在这不只是 synthetic reproduction。 我找到了提交 #2918 之前真实运行 189Cloud PC 时保存下来的 OpenList 日志。从 18:54 到 19:32，同类 parser error 连续出现了很多次，例如： 这批真实日志中的 parser input 始终只有一个 结合当时确定执行的： time.ParseInLocation(f, bs+" +08", time.Local)至少在我实际遇到并修复的这个 189Cloud PC 场景中，追加前的 而不是： 否则旧代码构造出的 parser input 必然包含： 但我保存的所有这些真实请求日志中，都没有出现 这不是 synthetic test，也不只是我根据 #2917 猜测。它是提交 #2918 前真实 189Cloud PC 环境中的连续运行日志。 我不会把这些日志夸张成 raw HTTP dump，也不主张 189Cloud 永远不可能返回自带 timezone 的字符串。#2919 对 raw 自带 timezone 的输入增加兼容，仍然可以算额外 coverage。 但这些日志已经足以反驳 AI 报告中的绝对结论：
至少在我真实遇到并修复的这批请求中，实际 parser input 与这个前提不一致。 #2918 中对应的 comma、12-hour 和 U+202F regression test，也正是根据这批真实运行日志加入的，而不是报告所描述的无依据测试。 提交 #2918 前保存的全部相关 parser 日志3. AI 报告错误引用了 #2918 的测试报告把 #2918 的 legacy test 写成： 但 #2918 实际测试的 expected hour 是 15，不是 23： {
name: "legacy month date",
input: "Aug 12, 2026 15:32:44 PM",
want: time.Date(2026, time.August, 12, 15, 32, 44, 0, time.Local),
},这个输入语义上确实不规范，但对应 layout 来自共同 base 中原本就存在的： #2918 保留这个 case，是为了验证既有 parser 接受范围没有被无意改变；这与本次新增的正确 comma + 12-hour layout 是两个不同问题。 可以讨论是否应该顺便删除或修改这个 legacy layout，但不能把实际 expected value从 15 改写成 23，再据此评价 #2918 的测试行为。 另外，报告只列出了 #2919 的 invalid negative test，却遗漏了 #2918 的 189pc 和 189_tv tests 中同样存在： func TestTimeUnmarshalRejectsInvalidTime(t *testing.T) {
var got Time
if err := got.Unmarshal([]byte("Aug 12, 2026, 25:32:44 AM")); err == nil {
t.Fatal("Time.Unmarshal accepted an invalid time")
}
}所以这份对比报告在评价双方 negative test 覆盖时并不完整。 4. 技术偏好仍然没有回答 review 路径问题即使暂时不考虑上述事实错误，回答：
最多只能解释为什么你更喜欢 #2919 当时的实现。 它仍然没有回答我真正问的问题：
实现偏好和 review 流程是否对等，是两个不同问题。重复说明“#2919 更优”，不能代替对上述流程问题的回答。 5. 我没有要求在无代码复用证据的情况下追加
|
|
如果AI的评审有误，你可以友好的指出；如果您认为您的代码更优，也可以阐述原因 |
2 participants
Summary / 摘要
Add compatibility for the updated time strings returned by the 189Cloud PC and TV clients.
U+202F) and no-break space (U+00A0) before parsing.Jan 2, 2006, 3:04:05 PM -07for the current response format.The 189Cloud API may now return values such as
Aug 12, 2026, 3:32:44 AM. The existing parsers did not accept the comma after the year or the Unicode space beforeAM/PM. The parsing error can propagate to callers and may cause an otherwise successful operation to be reported as failed.Real-world verification was performed against 189Cloud PC after opening this PR: uploads complete successfully and the previous time parsing error no longer occurs. The same parser compatibility update is also applied to
189_tv, which uses the equivalent time parsing logic./ 此 PR 包含破坏性变更。
/ 此 PR 修改了公开 API、配置、存储格式或迁移行为。
/ 此 PR 需要关联仓库同步修改。
Related repository PRs / 关联仓库 PR:
Related Issues / 关联 Issue
Fixes #2917
Testing / 测试
go test -vet=off ./drivers/189pcgo test -vet=off ./drivers/189_tvgo test -vet=off ./drivers/189pc ./drivers/189_tv ./internal/model ./internal/op ./internal/fs ./pkg/utilsgo test ./...— FAILED due to pre-existing vet, environment, and service-availability failures described below.The full command was run, but it did not pass for reasons unrelated to this change:
drivers/189pc/utils.go:358anddrivers/189pc/utils.go:418.internal/netenvironment-sensitive failure:TestNewOSSClientUsesEnvironmentHTTPSProxyexpected*http.Transportbut received*net.safeTransport.pkg/aria2/rpctests could not connect to the required local service atlocalhost:6800.Checklist / 检查清单
/ 我已阅读 CONTRIBUTING。
/ 我确认此贡献符合仓库许可证、贡献规范和行为准则。
gofmt,go fmt, orprettierwhere applicable./ 我已按适用情况使用
gofmt、go fmt或prettier格式化变更代码。/ 我已在适用情况下请求相关维护者或代码所有者审查。
AI Disclosure / AI 使用声明
/ 此 PR 包含 AI 辅助内容。
Tools used / 使用工具:
Usage scope / 使用范围:
Code generation / 代码生成
Refactoring / 重构
Documentation / 文档
Tests / 测试
Translation / 翻译
Review assistance / 审查辅助
I have reviewed and validated all AI-assisted content included in this PR.
/ 我已审核并验证此 PR 中的所有 AI 辅助内容。
I have ensured that all AI-assisted commits include
Co-Authored-Byattribution./ 我已确保所有 AI 辅助提交都包含
Co-Authored-By归属信息。I can reproduce all AI-assisted content included in this PR without any AI tools.
/ 我可以在没有任何 AI 工具的情况下重现此 PR 中包含的所有 AI 辅助内容。