Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DXCDT-396: Standardize error messages #943

Merged
merged 5 commits into from
Dec 19, 2023
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
6 changes: 3 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ linters:
- whitespace
- goimports
- gosimple
# - errcheck
- errcheck
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Took the opportunity to update the golangci lint config as well to help with err checks, error messages and as well fix comments.

- unconvert
- gocritic
# - gosec
Expand All @@ -28,8 +28,9 @@ linters-settings:
staticcheck:
checks: [ "all" ]
godot:
scope: declarations
scope: all
capital: true
period: true
goimports:
local-prefixes: "github.com/auth0/auth0-cli"

Expand All @@ -40,5 +41,4 @@ issues:
- "package comment should be of the form"
- "should have comment"
- "be unexported"
- "error strings should not be capitalized or end with punctuation or a newline"
- "blank-imports"
14 changes: 8 additions & 6 deletions internal/analytics/analytics.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (t *Tracker) Wait(ctx context.Context) {
}()

select {
case <-ch: // waitgroup is done
case <-ch: // Waitgroup is done.
return
case <-ctx.Done():
return
Expand Down Expand Up @@ -101,9 +101,11 @@ func (t *Tracker) sendEvent(event *event) {
return
}

// defers execute in LIFO order
// Defers execute in LIFO order.
defer t.wg.Done()
defer resp.Body.Close()
defer func() {
_ = resp.Body.Close()
}()
}

func newEvent(eventName string, id string) *event {
Expand Down Expand Up @@ -131,9 +133,9 @@ func generateEventName(command string, action string) string {
}

switch length := len(commands); {
case length == 1: // the root command
case length == 1: // The root command.
return fmt.Sprintf("%s - %s - %s", eventNamePrefix, commands[0], action)
case length == 2: // a top-level command e.g. auth0 apps
case length == 2: // A top-level command e.g. auth0 apps.
return fmt.Sprintf("%s - %s - %s - %s", eventNamePrefix, commands[0], commands[1], action)
case length >= 3:
return fmt.Sprintf("%s - %s - %s - %s", eventNamePrefix, commands[1], strings.Join(commands[2:], " "), action)
Expand All @@ -143,7 +145,7 @@ func generateEventName(command string, action string) string {
}

func shouldTrack() bool {
if os.Getenv("AUTH0_CLI_ANALYTICS") == "false" || buildinfo.Version == "" { // Do not track debug builds
if os.Getenv("AUTH0_CLI_ANALYTICS") == "false" || buildinfo.Version == "" { // Do not track debug builds.
return false
}

Expand Down
2 changes: 1 addition & 1 deletion internal/analytics/analytics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestGenerateRunEventName(t *testing.T) {
func TestNewEvent(t *testing.T) {
t.Run("creates a new event instance", func(t *testing.T) {
event := newEvent("event", "id")
// Assert that the interval between the event timestamp and now is within 1 second
// Assert that the interval between the event timestamp and now is within 1 second.
assert.WithinDuration(t, time.Now(), time.Unix(0, event.Timestamp*int64(1000000)), 1*time.Second)
assert.Equal(t, event.App, appID)
assert.Equal(t, event.Event, "event")
Expand Down
10 changes: 3 additions & 7 deletions internal/ansi/ansi_test.go
Original file line number Diff line number Diff line change
@@ -1,22 +1,18 @@
package ansi

import (
"os"
"testing"

"github.com/stretchr/testify/assert"
)

func TestShouldUseColors(t *testing.T) {
os.Setenv("CLICOLOR_FORCE", "true")
t.Setenv("CLICOLOR_FORCE", "true")
assert.True(t, shouldUseColors())

os.Setenv("CLICOLOR_FORCE", "0")
t.Setenv("CLICOLOR_FORCE", "0")
assert.False(t, shouldUseColors())

os.Unsetenv("CLI_COLOR_FORCE")

os.Setenv("CLICOLOR", "0")
t.Setenv("CLICOLOR", "0")
assert.False(t, shouldUseColors())
os.Unsetenv("CLICOLOR")
}
10 changes: 7 additions & 3 deletions internal/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ func WaitUntilUserLogsIn(ctx context.Context, httpClient *http.Client, state Sta
if err != nil {
return Result{}, fmt.Errorf("cannot get device code: %w", err)
}
defer r.Body.Close()
defer func() {
_ = r.Body.Close()
}()

var res struct {
AccessToken string `json:"access_token"`
Expand Down Expand Up @@ -118,7 +120,7 @@ func WaitUntilUserLogsIn(ctx context.Context, httpClient *http.Client, state Sta

var RequiredScopes = []string{
"openid",
"offline_access", // for retrieving refresh token
"offline_access", // For retrieving refresh token.
"create:clients", "delete:clients", "read:clients", "update:clients",
"read:client_grants",
"create:resource_servers", "delete:resource_servers", "read:resource_servers", "update:resource_servers",
Expand Down Expand Up @@ -167,7 +169,9 @@ func GetDeviceCode(ctx context.Context, httpClient *http.Client, additionalScope
if err != nil {
return State{}, fmt.Errorf("failed to send the request: %w", err)
}
defer response.Body.Close()
defer func() {
_ = response.Body.Close()
}()

if response.StatusCode != http.StatusOK {
bodyBytes, err := io.ReadAll(response.Body)
Expand Down
18 changes: 12 additions & 6 deletions internal/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestWaitUntilUserLogsIn(t *testing.T) {
Expand All @@ -34,12 +35,14 @@ func TestWaitUntilUserLogsIn(t *testing.T) {
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
if counter < 1 {
io.WriteString(w, `{
_, err := io.WriteString(w, `{
"error": "authorization_pending",
"error_description": "still pending auth"
}`)
require.NoError(t, err)
} else {
io.WriteString(w, tokenResponse)
_, err := io.WriteString(w, tokenResponse)
require.NoError(t, err)
}
counter++
}))
Expand Down Expand Up @@ -89,7 +92,8 @@ func TestWaitUntilUserLogsIn(t *testing.T) {
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(testCase.httpStatus)
if testCase.response != "" {
io.WriteString(w, testCase.response)
_, err := io.WriteString(w, testCase.response)
require.NoError(t, err)
}
}))

Expand All @@ -111,13 +115,14 @@ func TestGetDeviceCode(t *testing.T) {
t.Run("successfully retrieve state from response", func(t *testing.T) {
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
io.WriteString(w, `{
_, err := io.WriteString(w, `{
"device_code": "device-code-here",
"user_code": "user-code-here",
"verification_uri_complete": "verification-uri-here",
"expires_in": 1000,
"interval": 1
}`)
require.NoError(t, err)
}))

defer ts.Close()
Expand Down Expand Up @@ -163,7 +168,8 @@ func TestGetDeviceCode(t *testing.T) {
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(testCase.httpStatus)
if testCase.response != "" {
io.WriteString(w, testCase.response)
_, err := io.WriteString(w, testCase.response)
require.NoError(t, err)
}
}))

Expand Down Expand Up @@ -201,7 +207,7 @@ func TestParseTenant(t *testing.T) {
},
{
name: "bad json encoding",
accessToken: "Zm9v.Zm9v", // foo encoded in base64
accessToken: "Zm9v.Zm9v", // Foo encoded in base64.
err: "invalid character 'o' in literal false (expecting 'a')",
},
{
Expand Down
12 changes: 9 additions & 3 deletions internal/auth/authutil/browser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ func TestWaitForBrowserCallback(t *testing.T) {
assert.NoError(t, err)
body := string(bytes)

defer resp.Body.Close()
defer func() {
_ = resp.Body.Close()
}()
assert.Contains(t, body, "You can close the window and go back to the CLI to see the user info and tokens.")
})

Expand All @@ -43,7 +45,9 @@ func TestWaitForBrowserCallback(t *testing.T) {
bytes, err := io.ReadAll(resp.Body)
assert.NoError(t, err)
body := string(bytes)
defer resp.Body.Close()
defer func() {
_ = resp.Body.Close()
}()

assert.Contains(t, body, "Unable to extract code from request, please try authenticating again")
})
Expand All @@ -65,7 +69,9 @@ func TestWaitForBrowserCallback(t *testing.T) {

time.Sleep(1 * time.Second)

defer s.Close()
defer func() {
_ = s.Close()
}()

code, state, callbackErr := WaitForBrowserCallback("localhost:1234")

Expand Down
4 changes: 3 additions & 1 deletion internal/auth/authutil/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ func ExchangeCodeForToken(httpClient *http.Client, baseDomain, clientID, clientS
if err != nil {
return nil, fmt.Errorf("unable to exchange code for token: %w", err)
}
defer r.Body.Close()
defer func() {
_ = r.Body.Close()
}()

if r.StatusCode != http.StatusOK {
return nil, fmt.Errorf("unable to exchange code for token: %s", r.Status)
Expand Down
7 changes: 5 additions & 2 deletions internal/auth/authutil/exchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,20 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestExchangeCodeForToken(t *testing.T) {
t.Run("Successfully call token endpoint", func(t *testing.T) {
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
io.WriteString(w, `{
_, err := io.WriteString(w, `{
"access_token": "access-token-here",
"id_token": "id-token-here",
"token_type": "token-type-here",
"expires_in": 1000
}`)
require.NoError(t, err)
}))

defer ts.Close()
Expand Down Expand Up @@ -59,7 +61,8 @@ func TestExchangeCodeForToken(t *testing.T) {
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(testCase.httpStatus)
if testCase.response != "" {
io.WriteString(w, testCase.response)
_, err := io.WriteString(w, testCase.response)
require.NoError(t, err)
}
}))

Expand Down
4 changes: 3 additions & 1 deletion internal/auth/authutil/user_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ func FetchUserInfo(httpClient *http.Client, baseDomain, token string) (*UserInfo
if err != nil {
return nil, fmt.Errorf("unable to exchange code for token: %w", err)
}
defer res.Body.Close()
defer func() {
_ = res.Body.Close()
}()

if res.StatusCode != http.StatusOK {
return nil, fmt.Errorf("unable to fetch user info: %s", res.Status)
Expand Down
10 changes: 7 additions & 3 deletions internal/auth/authutil/user_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestUserInfo(t *testing.T) {
Expand All @@ -16,7 +17,8 @@ func TestUserInfo(t *testing.T) {
assert.Equal(t, "Bearer token", r.Header.Get("authorization"))

w.Header().Set("Content-Type", "application/json")
io.WriteString(w, `{"name": "Joe Bloggs","email_verified":true}`)
_, err := io.WriteString(w, `{"name": "Joe Bloggs","email_verified":true}`)
require.NoError(t, err)
}))

defer ts.Close()
Expand All @@ -35,7 +37,8 @@ func TestUserInfo(t *testing.T) {
assert.Equal(t, "Bearer token", r.Header.Get("authorization"))

w.Header().Set("Content-Type", "application/json")
io.WriteString(w, `{"email_verified":"true"}`)
_, err := io.WriteString(w, `{"email_verified":"true"}`)
require.NoError(t, err)
}))

defer ts.Close()
Expand Down Expand Up @@ -78,7 +81,8 @@ func TestUserInfo(t *testing.T) {
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(testCase.httpStatus)
if testCase.response != "" {
io.WriteString(w, testCase.response)
_, err := io.WriteString(w, testCase.response)
require.NoError(t, err)
}
}))

Expand Down
5 changes: 4 additions & 1 deletion internal/auth/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ func RefreshAccessToken(httpClient *http.Client, tenant string) (TokenResponse,
return TokenResponse{}, fmt.Errorf("cannot get a new access token from the refresh token: %w", err)
}

defer r.Body.Close()
defer func() {
_ = r.Body.Close()
}()

if r.StatusCode != http.StatusOK {
b, _ := io.ReadAll(r.Body)
bodyStr := string(b)
Expand Down
2 changes: 1 addition & 1 deletion internal/auth0/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ type LogAPI interface {
// and descriptions, Log Data Event Listing.
List(ctx context.Context, opts ...management.RequestOption) (l []*management.Log, err error)

// Search is an alias for List
// Search is an alias for List.
Search(ctx context.Context, opts ...management.RequestOption) ([]*management.Log, error)
}
12 changes: 8 additions & 4 deletions internal/auth0/quickstart.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ func (q Quickstart) Download(ctx context.Context, downloadPath string, client *m
if err := tmpFile.Close(); err != nil {
return err
}
defer os.Remove(tmpFile.Name())
defer func() {
_ = os.Remove(tmpFile.Name())
}()

if err := os.RemoveAll(downloadPath); err != nil {
return err
Expand All @@ -102,7 +104,9 @@ func GetQuickstarts(ctx context.Context) (Quickstarts, error) {
if err != nil {
return nil, err
}
defer response.Body.Close()
defer func() {
_ = response.Body.Close()
}()

if response.StatusCode != http.StatusOK {
return nil, fmt.Errorf(
Expand All @@ -126,7 +130,7 @@ func (q Quickstarts) FindByStack(stack string) (Quickstart, error) {
}
}

return Quickstart{}, fmt.Errorf("quickstart not found for %s", stack)
return Quickstart{}, fmt.Errorf("failed to find any quickstarts for stack: %q", stack)
}

func (q Quickstarts) FilterByType(qsType string) (Quickstarts, error) {
Expand All @@ -138,7 +142,7 @@ func (q Quickstarts) FilterByType(qsType string) (Quickstarts, error) {
}

if len(filteredQuickstarts) == 0 {
return nil, fmt.Errorf("unable to find any quickstarts for: %s", qsType)
return nil, fmt.Errorf("failed to find any quickstarts for type: %q", qsType)
}

return filteredQuickstarts, nil
Expand Down
Loading
Loading