[client] Answer terminalLoginError's nil case on its own terms

A successful Login reaches terminalLoginError with a nil error, and nothing
covered that. It happens to work on grpc v1.80.0 — gstatus.FromError(nil)
answers (nil, true), and Status.Code tolerates a nil receiver by returning
codes.OK, which is not in the terminal set — but that is a chain of internal
details to be relying on for the common path, and none of it was asserted.

Now the nil error is handled where it is obvious, and the table covers it.

Reported by CodeRabbit on PR #7398, which called it a panic; measured on
v1.80.0 it is not one. The gap was the untested reliance, not a crash.
This commit is contained in:
riccardom
2026-09-02 18:06:23 +02:00
parent 34cae56ee3
commit 0bfe2f6ead
2 changed files with 9 additions and 0 deletions
+8
View File
@@ -299,6 +299,14 @@ func DialClientGRPCServer(ctx context.Context, addr string) (*grpc.ClientConn, e
// they each carried their own copy of this list — which is how one of them
// ended up retrying a refusal the other treated as final.
func terminalLoginError(err error) bool {
// A successful Login reaches here with a nil error, and that is not a
// terminal failure. Handled explicitly rather than left to
// gstatus.FromError, which answers (nil, true) for a nil error and leans on
// Status.Code tolerating a nil receiver to come back as codes.OK.
if err == nil {
return false
}
s, ok := gstatus.FromError(err)
if !ok {
return false
+1
View File
@@ -74,6 +74,7 @@ func TestTerminalLoginError(t *testing.T) {
{name: "daemon unreachable, worth retrying", err: gstatus.Errorf(codes.Unavailable, "connection refused"), wantTerminal: false},
{name: "transient internal failure", err: gstatus.Errorf(codes.Internal, "boom"), wantTerminal: false},
{name: "not a status error", err: errors.New("boom"), wantTerminal: false},
{name: "no error at all, the login succeeded", err: nil, wantTerminal: false},
}
for _, tt := range tests {