AlexStocks commented on code in PR #138:
URL: https://github.com/apache/dubbo-getty/pull/138#discussion_r3642579550
##########
server_test.go:
##########
@@ -226,6 +226,30 @@ func TestWSServeWSRequestClosesSelfConnectConn(t
*testing.T) {
}
}
+func TestWSSServerCloseDoesNotPanic(t *testing.T) {
+ certFile := filepath.Join(t.TempDir(), "server.crt")
+ keyFile := filepath.Join(t.TempDir(), "server.key")
Review Comment:
[P1] 补一条 unexpected Serve error 仍会失败的回归测试
当前用例只证明正常 `Close()` 不再触发 panic，没有保护本 PR 同时承诺保留的 fatal-error 路径。我在独立副本中把
`isHTTPServeCloseError` 变异为无条件返回 `true` 后，这个测试重复 20 次和全仓 `go test ./...`
仍全部通过，也就是所有 WSS Serve 故障被静默降级的回归无法被发现。请用可注入的 fake listener，必要时放到子进程中，分别验证
`http.ErrServerClosed`/wrapped `net.ErrClosed` 正常退出，而普通 fatal Accept/Serve
error 必须进入 error 日志并保持 panic。
##########
server.go:
##########
@@ -421,11 +422,29 @@ func (s *server) runWSEventLoop(newSession
NewSessionCallback) {
s.lock.Unlock()
err = server.Serve(s.streamListener)
if err != nil {
- log.Errorf("http.server.Serve(addr{%s}) = err:%+v",
s.addr, perrors.WithStack(err))
+ if isHTTPServeCloseError(err) {
+ s.logHTTPServeClose(err)
+ } else {
+ s.logHTTPServeError(err)
+ }
}
}()
}
+func (s *server) logHTTPServeClose(err error) {
+ log.Infof("http.server.Serve(addr{%s}) closed: %v", s.addr, err)
+}
+
+func isHTTPServeCloseError(err error) bool {
+ return errors.Is(err, http.ErrServerClosed) ||
+ errors.Is(err, net.ErrClosed) ||
+ strings.Contains(err.Error(), "use of closed network
connection")
Review Comment:
[P2] 不要仅按错误文本判定正常关闭
这里的 substring 会把不属于关闭的真实故障一起吞掉。我用真实 `http.Server.Serve` 和故障 listener 返回
`fatal listener failure after use of closed network connection marker`，当前
helper 会判为正常关闭，WSS loop 因而跳过 `logHTTPServeError` 和 panic。仓库最低 Go 版本已能通过
`errors.Is(err, net.ErrClosed)` 识别标准 listener 关闭；建议移除文本匹配，或者至少与明确的 shutdown
状态/sentinel 链联合约束，并补对应分类测试。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
