CAICAIIs opened a new issue, #3753: URL: https://github.com/apache/dubbo-go/issues/3753
### What happened `go test ./protocol/triple/triple_protocol/` fails intermittently on `develop`. The failures start at `302aa0e8`, the commit that enabled the 32 KiB streaming write buffer by default (#3749), and the parent commit is clean. CI hit this on an unrelated PR: the `Unit Test` job of [run 36395309645](https://github.com/apache/dubbo-go/actions/runs/36395309645/job/108840405288) failed with ``` --- FAIL: TestServer/http1/grpc/proto/cumsum_cancel_before_send --- FAIL: TestServer/http1/grpc/json_gzip/cumsum ``` while the same PR only adds a unary `$invoke` handler registration for IDL exports and does not touch the streaming path. ### Environment - `apache/dubbo-go` `develop` at `302aa0e8`, baseline `461b222` (`302aa0e8^`) - linux/amd64, Go 1.25.9 (the same failures show up on darwin/arm64, Go 1.26.1) - No external services needed, every case is an in-process `httptest` server ### Reproduction ``` git checkout 302aa0e8 go test ./protocol/triple/triple_protocol/ -count=10 ``` | Revision | Package runs | Runs with at least one failure | Failing tests in total | | --- | --- | --- | --- | | `302aa0e8` (#3749 merged) | 10 | 1 | 6 | | `302aa0e8` | 10 | 1 | 8 | | `461b222` (`302aa0e8^`) | 10 | 0 | 0 | | `461b222` | 10 | 0 | 0 | A third `-count=10` run on `461b222` was also clean, so 30 consecutive package runs are green before #3749 and roughly one in five runs fails after it. Narrowed to the two tests that fail most often: ``` go test ./protocol/triple/triple_protocol/ -run 'TestStreamForServer|TestGRPCMissingTrailersError' -count=20 ``` fails 5 times in 20 runs. ### Failing assertions All of them are in `protocol/triple/triple_protocol/triple_ext_test.go`: | Test | Line | Assertion | Observed | | --- | --- | --- | --- | | `TestStreamForServer/nil-message` | 1692 | `assert.Nil(t, stream.CloseRequest())` | `unknown: EOF` | | `TestStreamForServer/get-spec` | 1711 | `assert.Nil(t, stream.CloseRequest())` | `unknown: EOF` | | `TestBidiOverHTTP1` | 2170 | `assert.Nil(t, stream.CloseRequest())` | `unknown: EOF` | | `TestUnflushableResponseWriter/grpc` | 2011 | `assert.Equal`, want `internal` | `unknown` (`unknown: EOF`) | Across two `-count=10` runs the failures were `TestStreamForServer` (7, subtests `nil-message` and `get-spec`), `TestBidiOverHTTP1` (3), `TestUnflushableResponseWriter` (2), `TestGRPCMissingTrailersError/sum` (2). Every reproducible failure involves a stream whose peer already ended it: a nil message, a server stream that finished, or the bidi case over HTTP/1 where the response ends with a 505. The tests close their request side after receiving that end. ### Likely cause `grpcClientConn.CloseRequest` returns the write buffer's flush error as-is, while the flush path used before blocking on the response deliberately tolerates `io.EOF`: ```go // protocol/triple/triple_protocol/protocol_grpc.go:382 func (cc *grpcClientConn) CloseRequest() error { var flushErr error if cc.writeBuffer != nil { flushErr = cc.writeBuffer.Close() } closeErr := cc.call.CloseWrite() if flushErr != nil { return flushErr } return closeErr } ``` ```go // protocol/triple/triple_protocol/protocol_grpc.go:402 if err := cc.writeBuffer.Flush(); err != nil && !errors.Is(err, io.EOF) { // A closed request pipe can accompany a server response. Preserve // the response status instead of replacing it with the write-side EOF. cc.call.SetError(err) } ``` Before #3749 the buffer was absent, `flushErr` stayed nil, and `CloseRequest` returned only the `CloseWrite` result, which is nil once the peer has ended the stream. With buffering on by default, `CloseRequest` now flushes into a request pipe the server has already closed, and that `io.EOF` is returned as `unknown: EOF`. `TestUnflushableResponseWriter/grpc` shows the same family of problem from the other side: the write-side error replaces the code the test expects (`internal`) with `unknown`. The change also touched `buffered_writer.go`, `duplex_http_call.go` and `client_stream.go`, and its own message notes that the close lifecycle tests were left on the unbuffered path "because their setup waits for the server to receive the first Send", which is the shape of the tests that are now failing. ### Impact Beyond the red CI on unrelated PRs, this is not only a test problem: any caller that closes the request side of a stream whose peer already responded now receives `unknown: EOF` from `CloseRequest` where it previously received nil. ### Possible directions - tolerate `io.EOF` in `CloseRequest` the same way the flush path does, or - keep the flush error but not at the cost of the response status when the stream already produced one, and - decide whether `assert.Nil(t, stream.CloseRequest())` is still the right expectation in the affected tests. @lizining1231 as the author of #3749, could you take a look? I am happy to prepare a fix if you would rather have someone else pick it up. -- 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]
