lizining1231 opened a new issue, #3670: URL: https://github.com/apache/dubbo-go/issues/3670
### Summary In `protocol/triple`, a single-protocol (HTTP/2 or HTTP/3) server leaks `http.ErrServerClosed` from `Server.Run` after a normal `Stop`. The outer `startTransport` goroutine treats any non-nil error as a serve failure and logs `[Triple][Server] server serve failed, err=http: Server closed` every time the server is shut down normally — spurious error logs and monitoring noise. The dual-protocol path (`startHttp2AndHttp3`) already filters `http.ErrServerClosed` inside its errgroup, so the single-protocol and dual-protocol paths behave inconsistently. ### Affected Locations | Package | Test | Test Location | Related Source | | --- | --- | --- | --- | | protocol/triple/triple_protocol | `TestServer_HTTP2_StartAndStop` / `TestServer_HTTP3_StartAndStop` / `TestServer_HTTP2_StartAndGracefulStop` / `TestServer_HTTP3_StartAndGracefulStop` / `TestServer_RepeatedStartStop` | https://github.com/apache/dubbo-go/blob/master/protocol/triple/triple_protocol/server_lifecycle_test.go#L170-L203 | Single-protocol: https://github.com/apache/dubbo-go/blob/master/protocol/triple/triple_protocol/server.go#L201-L220 (`startHttp2` returns err directly) and `#L222-L250` (`startHttp3`); Logging: https://github.com/apache/dubbo-go/blob/master/protocol/triple/server.go#L230-L241 (`startTransport` goroutine logs `serve failed` on any non-nil error) | ### Reproduction ```bash # single-protocol HTTP/2 server, then Stop: go test -run 'TestServer_HTTP2_StartAndStop' ./protocol/triple/triple_protocol/ # observed: Run returns http.ErrServerClosed after a normal Stop (test asserts it today), # which makes the outer goroutine log "server serve failed, err=http: Server closed" ``` The existing tests currently **assert** the leak (`require.ErrorIs(..., http.ErrServerClosed)`), i.e. they pin the buggy behavior as expected instead of asserting a clean shutdown. The dual-protocol sibling test asserts `require.NoError(...)` — the inconsistency is baked into the test suite. ### Root Cause - `startHttp2` (`server.go#L201-L220`) and `startHttp3` (`server.go#L222-L250`) return the `ListenAndServe` error verbatim, without filtering `http.ErrServerClosed` — the normal-shutdown signal from net/http. - `startHttp2AndHttp3` (`server.go#L292-L313`) already filters `err != http.ErrServerClosed` inside the errgroup, so dual-protocol `Run` returns `nil` after a normal stop. - `startTransport` (`protocol/triple/server.go#L230-L241`) logs `logger.Errorf("... server serve failed, err=%v", runErr)` for any non-nil `Run` error. - Net effect: single-protocol deployments log a spurious error on every normal shutdown, while dual-protocol deployments do not — inconsistent behavior across the three `Run` switch branches. ### Proposed Fix - Filter `http.ErrServerClosed` in `startHttp2` and `startHttp3` before returning, aligning them with `startHttp2AndHttp3`. Note: only `http.ErrServerClosed` is swallowed; genuine errors (e.g. `EADDRINUSE` on bind) still propagate. - Update the 6 assertions in `server_lifecycle_test.go` (L178/L189/L203/L233/L249/L340) from `require.ErrorIs(..., http.ErrServerClosed)` to `require.NoError(...)`. The `if/else` in `TestServer_RepeatedStartStop` then becomes uniform and can be simplified. - Verification: the updated tests fail on the pre-fix code (they assert `nil` while `Run` returns `ErrServerClosed`) and pass after the one-line fix; full `./protocol/triple/...` suite green. -- 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]
