Skip to content

Commit ec981d0

Browse files
authored
fix: suppress retry logs in version command when SpiceDB unavailable (#564)
1 parent 28a4697 commit ec981d0

2 files changed

Lines changed: 115 additions & 7 deletions

File tree

internal/cmd/version.go

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@ package cmd
33
import (
44
"fmt"
55
"os"
6+
"strconv"
67

78
"github.com/gookit/color"
89
"github.com/jzelinskie/cobrautil/v2"
910
"github.com/mattn/go-isatty"
11+
"github.com/rs/zerolog/log"
1012
"github.com/spf13/cobra"
1113
"google.golang.org/grpc"
1214
"google.golang.org/grpc/metadata"
@@ -63,18 +65,41 @@ func versionCmdFunc(cmd *cobra.Command, _ []string) error {
6365
_, err := client.GetCurrentTokenWithCLIOverride(cmd, configStore, secretStore)
6466

6567
if err == nil {
66-
spiceClient, err := client.NewClient(cmd)
67-
if err != nil {
68-
return err
68+
// Temporarily disable retries for version check to avoid noisy error logs
69+
// when SpiceDB is unavailable
70+
userSetRetries := cmd.Flags().Changed("max-retries")
71+
var originalValue uint
72+
73+
if !userSetRetries {
74+
originalValue, _ = cmd.Flags().GetUint("max-retries")
75+
_ = cmd.Flags().Set("max-retries", "0")
6976
}
70-
serverVersion, err := getServerVersion(cmd, spiceClient)
71-
if err != nil {
72-
return err
77+
78+
spiceClient, err := client.NewClient(cmd)
79+
80+
// Restore original max-retries value and Changed state
81+
if !userSetRetries {
82+
_ = cmd.Flags().Set("max-retries", strconv.FormatUint(uint64(originalValue), 10))
83+
cmd.Flags().Lookup("max-retries").Changed = false
7384
}
7485

7586
blue := color.FgLightBlue.Render
7687
fmt.Print(blue("service: "))
77-
console.Println(serverVersion)
88+
89+
if err != nil {
90+
// Log at debug level and continue with unknown version
91+
log.Debug().Err(err).Msg("failed to connect to SpiceDB")
92+
console.Println("(unknown)")
93+
} else {
94+
serverVersion, err := getServerVersion(cmd, spiceClient)
95+
if err != nil {
96+
// Log at debug level and continue with unknown version
97+
log.Debug().Err(err).Msg("failed to get server version")
98+
console.Println("(unknown)")
99+
} else {
100+
console.Println(serverVersion)
101+
}
102+
}
78103
}
79104
}
80105

internal/cmd/version_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,22 @@
11
package cmd
22

33
import (
4+
"bytes"
45
"context"
6+
"os"
57
"testing"
68

9+
"github.com/spf13/cobra"
710
"github.com/stretchr/testify/require"
811
"google.golang.org/grpc"
12+
"google.golang.org/grpc/codes"
913
"google.golang.org/grpc/metadata"
14+
"google.golang.org/grpc/status"
1015

1116
"github.com/authzed/authzed-go/pkg/responsemeta"
1217
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
1318

19+
"github.com/authzed/zed/internal/client"
1420
zedtesting "github.com/authzed/zed/internal/testing"
1521
)
1622

@@ -138,3 +144,80 @@ func (m *mockSchemaServiceClient) ReadSchema(_ context.Context, _ *v1.ReadSchema
138144
func (m *mockSchemaServiceClient) WriteSchema(_ context.Context, _ *v1.WriteSchemaRequest, _ ...grpc.CallOption) (*v1.WriteSchemaResponse, error) {
139145
return nil, nil
140146
}
147+
148+
func TestVersionCommandWithUnavailableServer(t *testing.T) {
149+
// Note: Not running in parallel because we modify globals (client.NewClient, os.Stdout, os.Stderr)
150+
151+
// This test verifies issue #554 is fixed: when SpiceDB is unavailable,
152+
// zed version should not produce noisy retry error logs
153+
154+
// Save and restore original client.NewClient
155+
originalNewClient := client.NewClient
156+
t.Cleanup(func() {
157+
client.NewClient = originalNewClient
158+
})
159+
160+
// Mock client.NewClient to return Unavailable error
161+
client.NewClient = func(_ *cobra.Command) (client.Client, error) {
162+
return nil, status.Error(codes.Unavailable, "connection refused")
163+
}
164+
165+
// Capture stdout to check output
166+
oldStdout := os.Stdout
167+
rOut, wOut, _ := os.Pipe()
168+
os.Stdout = wOut
169+
t.Cleanup(func() {
170+
os.Stdout = oldStdout
171+
})
172+
173+
// Capture stderr to verify no retry errors are logged
174+
oldStderr := os.Stderr
175+
rErr, wErr, _ := os.Pipe()
176+
os.Stderr = wErr
177+
t.Cleanup(func() {
178+
os.Stderr = oldStderr
179+
})
180+
181+
// Create command with all required flags
182+
cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t,
183+
zedtesting.BoolFlag{FlagName: "include-remote-version", FlagValue: true},
184+
zedtesting.BoolFlag{FlagName: "include-deps", FlagValue: false},
185+
zedtesting.UintFlag{FlagName: "max-retries", FlagValue: 10},
186+
zedtesting.StringFlag{FlagName: "endpoint", FlagValue: ""},
187+
zedtesting.StringFlag{FlagName: "token", FlagValue: ""},
188+
zedtesting.StringFlag{FlagName: "certificate-path", FlagValue: ""},
189+
zedtesting.StringFlag{FlagName: "hostname-override", FlagValue: ""},
190+
zedtesting.StringFlag{FlagName: "permissions-system", FlagValue: ""},
191+
zedtesting.StringFlag{FlagName: "proxy", FlagValue: ""},
192+
zedtesting.StringFlag{FlagName: "request-id", FlagValue: ""},
193+
zedtesting.BoolFlag{FlagName: "insecure", FlagValue: false},
194+
zedtesting.BoolFlag{FlagName: "no-verify-ca", FlagValue: false},
195+
zedtesting.BoolFlag{FlagName: "skip-version-check", FlagValue: false},
196+
zedtesting.IntFlag{FlagName: "max-message-size", FlagValue: 0},
197+
)
198+
199+
// Run the version command
200+
err := versionCmdFunc(cmd, nil)
201+
202+
// Close writers and read captured outputs
203+
wOut.Close()
204+
wErr.Close()
205+
var stdout bytes.Buffer
206+
_, _ = stdout.ReadFrom(rOut)
207+
var stderr bytes.Buffer
208+
_, _ = stderr.ReadFrom(rErr)
209+
210+
// Command should succeed despite unavailable server
211+
require.NoError(t, err)
212+
213+
// Stdout should contain client version and service: (unknown)
214+
output := stdout.String()
215+
require.Contains(t, output, "zed")
216+
require.Contains(t, output, "service:")
217+
require.Contains(t, output, "(unknown)")
218+
219+
// Stderr should be empty (no retry error logs)
220+
stderrOutput := stderr.String()
221+
require.NotContains(t, stderrOutput, "retrying gRPC call")
222+
require.NotContains(t, stderrOutput, "Unavailable")
223+
}

0 commit comments

Comments
 (0)