来自 #268 的架构级评审建议。不阻塞合入,仅供参考是否有更好的架构解法。
💡 [建议 · 兼容] 转发层 MaxRetries 语义可能被平滑忽略 cluster/peerpool.go:24
问题根因:PR 将 peerMaxRetries 设为 -1 以表达「不重试」,但 SDK 的 Options.applyDefaults() 把负值统一夹到 0,即不报错、静默降级为重试 0 次。设置 -1 与设置 0 在行为上几乎等价——区别仅在于是否在 future 版本里被忽略。这削弱了 PR 想表达的「转发绝不重试」意图的可验证性:代码里写 -1 在调用方看来是明确不重试,但实际是负值被默认值逻辑吞掉。
为什么低级解法不够:通用评审会建议把注释改清楚即可,但这没有解决「语义被静默夹掉」本身——将来若默认值逻辑变化,-1 的用意会漂移。应让 SDK 对负值给出显式语义,而非默默无差别化简。
架构级方案:建议 SDK 为『显式不重试』提供一条专门通道:例如允许 MaxRetries 设为 -1 时返回/记录一个哨兵错误『SDK 不支持显式禁止重试』,或者提供一个 EnableRetry(bool) 开关,让调用方表达『我刻意不要重试』而不是『让我撞上默认值』。更简单地,把 Options 的 applyDefaults 对负值的处理改为显式保留(如设置 0 时取默认、负值则保持不重试),并让文档、注释与行为严格一致。
代价/收益:代价:SDK 需要增加一项语义分支或一个 boolean 开关。收益:转发时『不重试』的意图不会被默认值逻辑悄悄改写,语义在白盒审计时可验证。
💡 [建议 · 兼容] 转发层 MaxRetries 语义可能被平滑忽略
cluster/peerpool.go:24问题根因:PR 将 peerMaxRetries 设为 -1 以表达「不重试」,但 SDK 的 Options.applyDefaults() 把负值统一夹到 0,即不报错、静默降级为重试 0 次。设置 -1 与设置 0 在行为上几乎等价——区别仅在于是否在 future 版本里被忽略。这削弱了 PR 想表达的「转发绝不重试」意图的可验证性:代码里写 -1 在调用方看来是明确不重试,但实际是负值被默认值逻辑吞掉。
为什么低级解法不够:通用评审会建议把注释改清楚即可,但这没有解决「语义被静默夹掉」本身——将来若默认值逻辑变化,-1 的用意会漂移。应让 SDK 对负值给出显式语义,而非默默无差别化简。
架构级方案:建议 SDK 为『显式不重试』提供一条专门通道:例如允许 MaxRetries 设为 -1 时返回/记录一个哨兵错误『SDK 不支持显式禁止重试』,或者提供一个 EnableRetry(bool) 开关,让调用方表达『我刻意不要重试』而不是『让我撞上默认值』。更简单地,把 Options 的 applyDefaults 对负值的处理改为显式保留(如设置 0 时取默认、负值则保持不重试),并让文档、注释与行为严格一致。
代价/收益:代价:SDK 需要增加一项语义分支或一个 boolean 开关。收益:转发时『不重试』的意图不会被默认值逻辑悄悄改写,语义在白盒审计时可验证。