internal/transport: convert ConnectionError to Unavailable status when writing headers
#6891
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #6891 +/- ##
==========================================
- Coverage 83.76% 83.75% -0.01%
==========================================
Files 287 287
Lines 30832 30835 +3
==========================================
+ Hits 25825 25827 +2
- Misses 3953 3954 +1
Partials 1054 1054
|
|
@mustafasen81 : Could you please sign the CLA before we proceed and review your PR. Thanks. |
|
@easwars, I signed the CLA. |
easwars
changed the title
ConnectionError to Unavailable status when writing headers
|
@dfawley : for second set of eyes |
internal/transport/transport_test.go
Outdated
| if len(server.conns) != 1 { | ||
| t.Fatalf("Server has %d connections from the client, want 1", len(server.conns)) | ||
| } | ||
|
|
||
| // Get the server transport for the connecton to the client. | ||
| var serverTransport *http2Server | ||
| server.mu.Lock() | ||
| for k := range server.conns { | ||
| serverTransport = k.(*http2Server) | ||
| } |
There was a problem hiding this comment.
Do we not need to take server.mu before reading server.conns (the len check on 2155)?
Why a for loop instead of simply serverTransport := server.conns[0].(*http2Server) (which I'd feel better about with server.mu held for the duration of checking len(server.conns) and assigning serverTransport)?
There was a problem hiding this comment.
server.conns is of type map[ServerTransport]net.Conn so we cannot access a key by an index.
There was a problem hiding this comment.
server.mu moved before reading server.conns (the len check on 2155)
…Unavailable status in WriteHeader
This is an optional checker that lets us know we need to update our generated code. #6916 should take care of it, and there's no need to update your PR. |
The current behavior when server sees a connection failure, when trying to write headers, is to return
UNKNOWNstatus code. This PR changes the behavior to instead returnUNAVAILABLEin this case, which is what would be generated by the client if it saw the connection failure when making the RPC.Fixes #6845
RELEASE NOTES:
UNAVAILABLEinstead ofUNKNOWNwhen underlying connection is broken