Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.

Add new rich error messages #150

Merged
merged 1 commit into from
Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions cmd/coder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ import (
"os"
"runtime"

"cdr.dev/coder-cli/internal/clog"
"cdr.dev/coder-cli/internal/cmd"
"cdr.dev/coder-cli/internal/x/xterminal"

"go.coder.com/flog"
)

// Using a global for the version so it can be set at build time using ldflags.
Expand All @@ -31,7 +30,8 @@ func main() {

stdoutState, err := xterminal.MakeOutputRaw(os.Stdout.Fd())
if err != nil {
flog.Fatal("set output to raw: %s", err)
clog.Log(clog.Fatal(fmt.Sprintf("set output to raw: %s", err)))
os.Exit(1)
}
defer func() {
// Best effort. Would result in broken terminal on window but nothing we can do about it.
Expand All @@ -42,6 +42,7 @@ func main() {
app.Version = fmt.Sprintf("%s %s %s/%s", version, runtime.Version(), runtime.GOOS, runtime.GOARCH)

if err := app.ExecuteContext(ctx); err != nil {
flog.Fatal("%v", err)
clog.Log(err)
os.Exit(1)
}
}
8 changes: 4 additions & 4 deletions coder-sdk/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
// ErrNotFound describes an error case in which the requested resource could not be found
var ErrNotFound = xerrors.Errorf("resource not found")

// apiError is the expected payload format for our errors.
type apiError struct {
// APIError is the expected payload format for our errors.
type APIError struct {
Err struct {
Msg string `json:"msg"`
} `json:"error"`
Expand All @@ -30,15 +30,15 @@ func (e *HTTPError) Error() string {
return fmt.Sprintf("dump response: %+v", err)
}

var msg apiError
var msg APIError
// Try to decode the payload as an error, if it fails or if there is no error message,
// return the response URL with the dump.
if err := json.NewDecoder(e.Response.Body).Decode(&msg); err != nil || msg.Err.Msg == "" {
return fmt.Sprintf("%s\n%s", e.Response.Request.URL, dump)
}

// If the payload was a in the expected error format with a message, include it.
return fmt.Sprintf("%s\n%s%s", e.Response.Request.URL, dump, msg.Err.Msg)
return msg.Err.Msg
}

func bodyError(resp *http.Response) error {
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ require (
github.com/rjeczalik/notify v0.9.2
github.com/spf13/cobra v1.0.0
github.com/stretchr/testify v1.6.1
go.coder.com/flog v0.0.0-20190906214207-47dd47ea0512
golang.org/x/crypto v0.0.0-20200422194213-44a606286825
golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a
golang.org/x/sys v0.0.0-20201018230417-eeed37f84f13
Expand Down
6 changes: 3 additions & 3 deletions internal/activity/pusher.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ package activity

import (
"context"
"fmt"
"time"

"golang.org/x/time/rate"

"cdr.dev/coder-cli/coder-sdk"

"go.coder.com/flog"
"cdr.dev/coder-cli/internal/clog"
)

const pushInterval = time.Minute
Expand Down Expand Up @@ -42,6 +42,6 @@ func (p *Pusher) Push(ctx context.Context) {
}

if err := p.client.PushActivity(ctx, p.source, p.envID); err != nil {
flog.Error("push activity: %s", err)
clog.Log(clog.Error(fmt.Sprintf("push activity: %s", err)))
}
}
131 changes: 131 additions & 0 deletions internal/clog/error.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package clog

import (
"errors"
"fmt"
"os"
"strings"

"github.com/fatih/color"
"golang.org/x/xerrors"
)

// CLIMessage provides a human-readable message for CLI errors and messages.
type CLIMessage struct {
Level string
Color color.Attribute
Header string
Lines []string
}

// CLIError wraps a CLIMessage and allows consumers to treat it as a normal error.
type CLIError struct {
CLIMessage
error
}

// String formats the CLI message for consumption by a human.
func (m CLIMessage) String() string {
var str strings.Builder
str.WriteString(fmt.Sprintf("%s: %s\n",
color.New(m.Color).Sprint(m.Level),
color.New(color.Bold).Sprint(m.Header)),
)
for _, line := range m.Lines {
str.WriteString(fmt.Sprintf(" %s %s\n", color.New(m.Color).Sprint("|"), line))
}
return str.String()
}

// Log logs the given error to stderr, defaulting to "fatal" if the error is not a CLIError.
// If the error is a CLIError, the plain error chain is ignored and the CLIError
// is logged on its own.
func Log(err error) {
var cliErr CLIError
if !xerrors.As(err, &cliErr) {
cliErr = Fatal(err.Error())
}
fmt.Fprintln(os.Stderr, cliErr.String())
}

// LogInfo prints the given info message to stderr.
func LogInfo(header string, lines ...string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused why LogInfo and LogSuccess are the only ones prefixed with Log

Copy link
Contributor Author

@cmoog cmoog Oct 21, 2020

Choose a reason for hiding this comment

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

Yeah this is definitely confusing. The issue I'm having is that Error, Warn, and Fatal return errors. So it would potentially be even more confusing for Info and Success to be the only ones with side-effects.

It gets pretty tricky when we actually do want to log errors as they happen: for example when iterating through a series of stop actions.

Any ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ohh i see now, I was skimming over the returns. Considering the others return errors this makes more sense

fmt.Fprint(os.Stderr, CLIMessage{
Level: "info",
Color: color.FgBlue,
Header: header,
Lines: lines,
}.String())
}

// LogSuccess prints the given info message to stderr.
func LogSuccess(header string, lines ...string) {
fmt.Fprint(os.Stderr, CLIMessage{
Level: "success",
Color: color.FgGreen,
Header: header,
Lines: lines,
}.String())
}

// Warn creates an error with the level "warning".
func Warn(header string, lines ...string) CLIError {
return CLIError{
CLIMessage: CLIMessage{
Color: color.FgYellow,
Level: "warning",
Header: header,
Lines: lines,
},
error: errors.New(header),
}
}

// Error creates an error with the level "error".
func Error(header string, lines ...string) CLIError {
return CLIError{
CLIMessage: CLIMessage{
Color: color.FgRed,
Level: "error",
Header: header,
Lines: lines,
},
error: errors.New(header),
}
}

// Fatal creates an error with the level "fatal".
func Fatal(header string, lines ...string) CLIError {
return CLIError{
CLIMessage: CLIMessage{
Color: color.FgRed,
Level: "fatal",
Header: header,
Lines: lines,
},
error: errors.New(header),
}
}

// Bold provides a convenience wrapper around color.New for brevity when logging.
func Bold(a string) string {
return color.New(color.Bold).Sprint(a)
}

// Tip formats according to the given format specifier and prepends a bolded "tip: " header.
func Tip(format string, a ...interface{}) string {
return fmt.Sprintf("%s %s", Bold("tip:"), fmt.Sprintf(format, a...))
}

// Hint formats according to the given format specifier and prepends a bolded "hint: " header.
func Hint(format string, a ...interface{}) string {
return fmt.Sprintf("%s %s", Bold("hint:"), fmt.Sprintf(format, a...))
}

// Cause formats according to the given format specifier and prepends a bolded "cause: " header.
func Cause(format string, a ...interface{}) string {
return fmt.Sprintf("%s %s", Bold("cause:"), fmt.Sprintf(format, a...))
}

// BlankLine is an empty string meant to be used in CLIMessage and CLIError construction.
const BlankLine = ""
6 changes: 5 additions & 1 deletion internal/cmd/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@ import (
"golang.org/x/xerrors"

"cdr.dev/coder-cli/coder-sdk"
"cdr.dev/coder-cli/internal/clog"
"cdr.dev/coder-cli/internal/config"
)

var errNeedLogin = xerrors.New("failed to read session credentials: did you run \"coder login\"?")
var errNeedLogin = clog.Fatal(
"failed to read session credentials",
clog.Hint(`did you run "coder login [https://coder.domain.com]"?`),
)

func newClient() (*coder.Client, error) {
sessionToken, err := config.Session.Read()
Expand Down
11 changes: 7 additions & 4 deletions internal/cmd/ceapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"

"cdr.dev/coder-cli/coder-sdk"
"cdr.dev/coder-cli/internal/clog"
"golang.org/x/xerrors"
)

Expand Down Expand Up @@ -73,10 +74,12 @@ func findEnv(ctx context.Context, client *coder.Client, envName, userEmail strin
found = append(found, env.Name)
}

return nil, notFoundButDidFind{
needle: envName,
haystack: found,
}
return nil, clog.Fatal(
"failed to find environment",
fmt.Sprintf("environment %q not found in %q", envName, found),
clog.BlankLine,
clog.Tip("run \"coder envs ls\" to view your environments"),
)
}

type notFoundButDidFind struct {
Expand Down
26 changes: 17 additions & 9 deletions internal/cmd/envs.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ package cmd

import (
"encoding/json"
"fmt"
"os"
"sync/atomic"

"cdr.dev/coder-cli/coder-sdk"
"cdr.dev/coder-cli/internal/clog"
"cdr.dev/coder-cli/internal/x/xtabwriter"
"github.com/spf13/cobra"
"golang.org/x/sync/errgroup"
"golang.org/x/xerrors"

"go.coder.com/flog"
)

func envsCommand() *cobra.Command {
Expand All @@ -37,7 +38,7 @@ func envsCommand() *cobra.Command {
return err
}
if len(envs) < 1 {
flog.Info("no environments found")
clog.LogInfo("no environments found")
return nil
}

Expand Down Expand Up @@ -92,26 +93,33 @@ coder envs --user charlie@coder.com ls -o json \
}

var egroup errgroup.Group
var fails int32
for _, envName := range args {
envName := envName
egroup.Go(func() error {
env, err := findEnv(cmd.Context(), client, envName, *user)
if err != nil {
flog.Error("failed to find environment by name \"%s\": %v", envName, err)
return xerrors.Errorf("find environment by name: %w", err)
atomic.AddInt32(&fails, 1)
clog.Log(err)
return xerrors.Errorf("find env by name: %w", err)
}

if err = client.StopEnvironment(cmd.Context(), env.ID); err != nil {
flog.Error("failed to stop environment \"%s\": %v", env.Name, err)
return xerrors.Errorf("stop environment: %w", err)
atomic.AddInt32(&fails, 1)
err = clog.Fatal(fmt.Sprintf("stop environment %q", env.Name),
clog.Cause(err.Error()), clog.BlankLine,
clog.Hint("current environment status is %q", env.LatestStat.ContainerStatus),
)
clog.Log(err)
return err
}
flog.Success("Successfully stopped environment %q", envName)
clog.LogSuccess(fmt.Sprintf("successfully stopped environment %q", envName))
return nil
})
}

if err = egroup.Wait(); err != nil {
return xerrors.Errorf("some stop operations failed")
return clog.Fatal(fmt.Sprintf("%d failure(s) emitted", fails))
}
return nil
},
Expand Down
5 changes: 2 additions & 3 deletions internal/cmd/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,13 @@ import (
"strings"

"cdr.dev/coder-cli/coder-sdk"
"cdr.dev/coder-cli/internal/clog"
"cdr.dev/coder-cli/internal/config"
"cdr.dev/coder-cli/internal/loginsrv"
"github.com/pkg/browser"
"github.com/spf13/cobra"
"golang.org/x/sync/errgroup"
"golang.org/x/xerrors"

"go.coder.com/flog"
)

func makeLoginCmd() *cobra.Command {
Expand Down Expand Up @@ -140,7 +139,7 @@ func login(cmd *cobra.Command, envURL *url.URL, urlCfg, sessionCfg config.File)
return xerrors.Errorf("store config: %w", err)
}

flog.Success("Logged in.")
clog.LogSuccess("logged in")

return nil
}
7 changes: 3 additions & 4 deletions internal/cmd/logout.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ package cmd
import (
"os"

"cdr.dev/coder-cli/internal/clog"
"cdr.dev/coder-cli/internal/config"
"github.com/spf13/cobra"
"golang.org/x/xerrors"

"go.coder.com/flog"
)

func makeLogoutCmd() *cobra.Command {
Expand All @@ -22,11 +21,11 @@ func logout(_ *cobra.Command, _ []string) error {
err := config.Session.Delete()
if err != nil {
if os.IsNotExist(err) {
flog.Info("no active session")
clog.LogInfo("no active session")
return nil
}
return xerrors.Errorf("delete session: %w", err)
}
flog.Success("logged out")
clog.LogSuccess("logged out")
return nil
}
Loading