You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "lizining1231 (via GitHub)" <gi...@apache.org> on 2026/09/28 12:50:29 UTC
[PR] Fix/issue 3753 buffered close eof [dubbo-go]
lizining1231 opened a new pull request, #3754:
URL: https://github.com/apache/dubbo-go/pull/3754
### Description
Fixes #3753
When a peer finishes a stream before the client closes its request side, the final buffered flush can return `io.EOF`. The gRPC and Triple `CloseRequest` paths returned that write-side EOF even when `CloseWrite` succeeded. As a result, callers could receive `unknown: EOF`, and the write-side error could replace the response status already sent by the peer.
Treat `io.EOF` from the final buffered flush as a completed peer-side close. The underlying write side is still closed, its close error is still returned, and non-EOF flush errors retain precedence.
Add a deterministic regression test for both wire formats. The test forces the buffered flush to return `io.EOF`; the server signals only after reading the request body to EOF, proving that `CloseRequest` still closes the write side without exposing the pipe error.
### Changes
- Update `grpcClientConn.CloseRequest` and `tripleUnaryClientConn.CloseRequest` to tolerate `io.EOF` from the final buffered flush. Both paths still call `CloseWrite`, return its result after a buffered EOF, and continue to report every non-EOF flush error.
- Add `TestCloseRequestIgnoresBufferedWriteEOF` to cover the fix on both gRPC and Triple. The test forces the final buffered flush to return `io.EOF` and verifies that `CloseRequest` succeeds while still closing the request write side.
### Verification
#### Deterministic regression test
The new `TestCloseRequestIgnoresBufferedWriteEOF` unit test writes pending bytes into a `streamBufferWriter` backed by an `errorWriter` that always returns `io.EOF`. This deterministically reproduces the final flush error without relying on client and server scheduling.
The buffer is attached to a real `duplexHTTPCall` connected to an `httptest.Server`. The server drains the request body with `io.Copy` and sends its completion signal only after reading EOF. The test therefore verifies both parts of the contract: `CloseRequest` does not expose the buffered EOF, and it still calls `CloseWrite` so the server can observe the end of the request body. Table-driven cases cover both `grpcClientConn` and `tripleUnaryClientConn`.
With the old `CloseRequest` condition restored, the new test fails deterministically:
```console
$ go test ./protocol/triple/triple_protocol/ -run '^TestCloseRequestIgnoresBufferedWriteEOF$' -count=1
--- FAIL: TestCloseRequestIgnoresBufferedWriteEOF (0.01s)
buffered_writer_test.go:525: CloseRequest after buffered EOF: EOF
FAIL
FAIL dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol 0.041s
FAIL
```
After the fix, the new regression and the adjacent error-handling tests pass 20 consecutive runs:
```console
$ go test ./protocol/triple/triple_protocol/ -run 'TestCloseRequestIgnoresBufferedWriteEOF|TestCloseRequestClosesWriteSideAfterFlushFailure|TestBufferedWriteEOFDoesNotMaskResponse' -count=20
ok dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol 0.649s
```
`TestCloseRequestClosesWriteSideAfterFlushFailure` remains in this test set to verify that non-EOF flush errors are still returned rather than being swallowed.
#### Issue reproduction
The narrowed reproduction from #3753 also failed before the fix:
```console
$ go test ./protocol/triple/triple_protocol/ -run 'TestStreamForServer|TestGRPCMissingTrailersError' -count=20
--- FAIL: TestGRPCMissingTrailersError (0.00s)
--- FAIL: TestGRPCMissingTrailersError/sum (0.00s)
triple_ext_test.go:890:
assertion: assert.Equal
got: unknown
want: internal
FAIL
FAIL dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol 2.107s
FAIL
```
After the fix, the test is ok
```console
$ for run in 1 2 3 4 5; do
> go test ./protocol/triple/triple_protocol/ -run 'TestStreamForServer|TestGRPCMissingTrailersError' -count=20
> done
ok dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol 2.075s
ok dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol 2.099s
ok dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol 2.081s
ok dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol 2.091s
ok dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol 2.098s
```
### Checklist
- [x] I confirm the target branch is `develop`
- [x] I have run `make fmt` to format my code
- [x] I have run `make test` to run local tests
- [x] I have added tests that prove my fix is effective or that my feature works
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): tolerate buffered EOF when closing requests [dubbo-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3754:
URL: https://github.com/apache/dubbo-go/pull/3754#issuecomment-5870331021
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3754?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `50.00000%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.52%. Comparing base ([`60d1c2a`](https://app.codecov.io/gh/apache/dubbo-go/commit/60d1c2a949f0ee0da4be3fe09fb79d295491e040?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`24453a2`](https://app.codecov.io/gh/apache/dubbo-go/commit/24453a2be6949cfb95ebfe8229ad5b26e3122d52?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 983 commits behind head on develop.
| [Files with missing lines](https://app.codecov.io/gh/apache/dubbo-go/pull/3754?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [protocol/triple/triple\_protocol/protocol\_triple.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3754?src=pr&el=tree&filepath=protocol%2Ftriple%2Ftriple_protocol%2Fprotocol_triple.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHJvdG9jb2wvdHJpcGxlL3RyaXBsZV9wcm90b2NvbC9wcm90b2NvbF90cmlwbGUuZ28=) | 0.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3754?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #3754 +/- ##
============================================
+ Coverage 46.76% 57.52% +10.76%
============================================
Files 295 487 +192
Lines 17172 39994 +22822
============================================
+ Hits 8031 23008 +14977
- Misses 8287 15202 +6915
- Partials 854 1784 +930
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/dubbo-go/pull/3754?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): tolerate buffered EOF when closing requests [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3754:
URL: https://github.com/apache/dubbo-go/pull/3754#issuecomment-5870212458
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3754) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3754&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3754&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3754&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3754&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3754&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3754)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): tolerate buffered EOF when closing requests [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl merged PR #3754:
URL: https://github.com/apache/dubbo-go/pull/3754
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): tolerate buffered EOF when closing requests [dubbo-go]
Posted by "lizining1231 (via GitHub)" <gi...@apache.org>.
lizining1231 commented on PR #3754:
URL: https://github.com/apache/dubbo-go/pull/3754#issuecomment-5870373018
当前ci错误或为与改动无关的网络问题
```
verifying go.mod:
cloud.google.com/go/bigquery@v1.53.0/go.mod
reading https://sum.golang.org/...
stream error: INTERNAL_ERROR; received from peer
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org