Skip to content

Commit

Permalink
fix(daemon): make status harmless. (#2987)
Browse files Browse the repository at this point in the history
  • Loading branch information
nathanhammond committed Dec 13, 2022
1 parent a5f7bc4 commit 3e50cc2
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 20 deletions.
16 changes: 12 additions & 4 deletions cli/internal/daemon/connector/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ import (

var (
// ErrFailedToStart is returned when the daemon process cannot be started
ErrFailedToStart = errors.New("daemon could not be started")
errVersionMismatch = errors.New("daemon version does not match client version")
ErrFailedToStart = errors.New("daemon could not be started")
// ErrVersionMismatch is returned when the daemon process was spawned by a different version than the connecting client
ErrVersionMismatch = errors.New("daemon version does not match client version")
errConnectionFailure = errors.New("could not connect to daemon")
// ErrTooManyAttempts is returned when the client fails to connect too many times
ErrTooManyAttempts = errors.New("reached maximum number of attempts contacting daemon")
Expand All @@ -38,6 +39,7 @@ var (
type Opts struct {
ServerTimeout time.Duration
DontStart bool // if true, don't attempt to start the daemon
DontKill bool // if true, don't attempt to kill the daemon
}

// Client represents a connection to the daemon process
Expand Down Expand Up @@ -237,13 +239,19 @@ func (c *Connector) connectInternal(ctx context.Context) (*Client, error) {
if err := c.sendHello(ctx, client); err == nil {
// We connected and negotiated a version, we're all set
return client, nil
} else if errors.Is(err, errVersionMismatch) {
} else if errors.Is(err, ErrVersionMismatch) {
// We don't want to knock down a perfectly fine daemon in a status check.
if c.Opts.DontKill {
return nil, err
}

// We now know we aren't going to return this client,
// but killLiveServer still needs it to send the Shutdown request.
// killLiveServer will close the client when it is done with it.
if err := c.killLiveServer(ctx, client, serverPid); err != nil {
return nil, err
}
// Loops back around and tries again.
} else if errors.Is(err, errConnectionFailure) {
// close the client, see if we can kill the stale daemon
_ = client.Close()
Expand Down Expand Up @@ -321,7 +329,7 @@ func (c *Connector) sendHello(ctx context.Context, client turbodprotocol.TurbodC
case codes.OK:
return nil
case codes.FailedPrecondition:
return errVersionMismatch
return ErrVersionMismatch
case codes.Unavailable:
return errConnectionFailure
default:
Expand Down
4 changes: 2 additions & 2 deletions cli/internal/daemon/connector/connector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ func TestKillLiveServer(t *testing.T) {
ClientConn: conn,
}
err = c.sendHello(ctx, client)
if !errors.Is(err, errVersionMismatch) {
t.Errorf("sendHello error got %v, want %v", err, errVersionMismatch)
if !errors.Is(err, ErrVersionMismatch) {
t.Errorf("sendHello error got %v, want %v", err, ErrVersionMismatch)
}
err = c.killLiveServer(ctx, client, 99999)
assert.NilError(t, err, "killLiveServer")
Expand Down
6 changes: 1 addition & 5 deletions cli/internal/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,5 @@ func GetClient(ctx context.Context, repoRoot turbopath.AbsoluteSystemPath, logge
LogPath: logPath,
TurboVersion: turboVersion,
}
client, err := c.Connect(ctx)
if err != nil {
return nil, err
}
return client, nil
return c.Connect(ctx)
}
26 changes: 17 additions & 9 deletions cli/internal/daemon/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ package daemon
import (
"context"
"encoding/json"
"errors"
"fmt"
"time"

"github.com/pkg/errors"
"github.com/vercel/turbo/cli/internal/cmdutil"
"github.com/vercel/turbo/cli/internal/daemon/connector"
"github.com/vercel/turbo/cli/internal/daemonclient"
Expand Down Expand Up @@ -34,6 +34,9 @@ func (l *lifecycle) status(ctx context.Context, outputJSON bool) error {
// If the daemon is not running, the status is that it's not running.
// We don't want to start it just to check the status.
DontStart: true,
// If the daemon is a different version simply report that.
// Don't attempt to kill the existing daemon.
DontKill: true,
})
if err != nil {
return l.reportStatusError(err, outputJSON)
Expand All @@ -60,22 +63,27 @@ func (l *lifecycle) status(ctx context.Context, outputJSON bool) error {
}

func (l *lifecycle) reportStatusError(err error, outputJSON bool) error {
var msg string
// Determine the unwrapped error message that we want to render.
var toRender error
if errors.Is(err, connector.ErrDaemonNotRunning) {
msg = "the daemon is not running"
toRender = connector.ErrDaemonNotRunning
} else if errors.Is(err, connector.ErrVersionMismatch) {
toRender = connector.ErrVersionMismatch
} else {
msg = err.Error()
toRender = err
}

// Spit it out as plain text or JSON.
if outputJSON {
rendered, err := json.MarshalIndent(map[string]string{
"error": msg,
rendered, jsonErr := json.MarshalIndent(map[string]string{
"error": toRender.Error(),
}, "", " ")
if err != nil {
return err
if jsonErr != nil {
return jsonErr
}
l.base.UI.Output(string(rendered))
} else {
l.base.UI.Output(fmt.Sprintf("Failed to contact daemon: %v", msg))
l.base.UI.Output(fmt.Sprintf("Failed to contact daemon: %v", toRender.Error()))
}
return nil
}

1 comment on commit 3e50cc2

@vercel
Copy link

@vercel vercel bot commented on 3e50cc2 Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.