Skip to content

Commit

Permalink
DXCDT-396: Standardize error messages (#943)
Browse files Browse the repository at this point in the history
  • Loading branch information
sergiught authored and Michael Christenson II committed Dec 19, 2023
1 parent a2550ed commit 364b643
Show file tree
Hide file tree
Showing 88 changed files with 496 additions and 441 deletions.
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
- 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
2 changes: 1 addition & 1 deletion internal/auth/authutil/browser.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func WaitForBrowserCallback(addr string) (code string, state string, err error)

if cb.code == "" {
_, _ = w.Write([]byte(resultPage("Login Failed",
"Unable to extract code from request, please try authenticating again.",
"Failed to extract code from request, please try authenticating again.",
"error-denied")))
} else {
_, _ = w.Write([]byte(resultPage("Login Successful",
Expand Down
14 changes: 10 additions & 4 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,9 +45,11 @@ 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")
assert.Contains(t, body, "Failed to extract code from request, please try authenticating again")
})

code, state, callbackErr := WaitForBrowserCallback("localhost:1234")
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)
}
Loading

0 comments on commit 364b643

Please sign in to comment.