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.

Sign up for GitHub

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

Jump to bottom

More detailed API key error #921

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

More detailed API key error #921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
090fbe2
Return a more detailed error for api keys if debug level is set
tomelm Jul 23, 2022
883c2d1
Fix expected error assertion
tomelm Jul 24, 2022
dc3552f
Remove stripe folder, path is different in windows
tomelm Jul 24, 2022
6a70d57
Check for windows specifically
tomelm Jul 25, 2022
cf7bfe2
Fix the windows check (oops)
tomelm Jul 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion pkg/config/profile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strings"
"time"

log "github.com/sirupsen/logrus"
"github.com/spf13/viper"

"github.com/stripe/stripe-cli/pkg/validators"
Expand Down Expand Up @@ -135,7 +136,7 @@ func (p *Profile) GetAPIKey(livemode bool) (string, error) {
p.RegisterAlias(TestModeAPIKeyName, "api_key")
}

if err := viper.ReadInConfig(); err == nil {
if err = viper.ReadInConfig(); err == nil {
key = viper.GetString(p.GetConfigField(TestModeAPIKeyName))
}
} else {
Expand All @@ -153,6 +154,13 @@ func (p *Profile) GetAPIKey(livemode bool) (string, error) {
return key, nil
}

// If the log level is debug, return the raw error message. otherwise
// return a generic configuration message
level := log.GetLevel()
if level == log.DebugLevel {
return "", err
}

return "", validators.ErrAPIKeyNotConfigured
}

Expand Down
40 changes: 40 additions & 0 deletions pkg/config/profile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ import (
"io/ioutil"
"os"
"path/filepath"
"runtime"
"testing"

"github.com/sirupsen/logrus"
"github.com/spf13/viper"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -101,6 +104,43 @@ test_mode_key_expires_at = '` + expiresAt + `'
cleanUp(c.ProfilesFile)
}

func TestAPIKeyLogLevel(t *testing.T) {
// Set the level to debug
logrus.SetLevel(logrus.DebugLevel)

c := &Config{
Color: "auto",
LogLevel: "debug",
Profile: Profile{
ProfileName: "tests",
TestModeAPIKey: "asdas",
},
ProfilesFile: "",
}

// For debug mode, the error should complain about a config file missing
// since we did not init the config
key, err := c.Profile.GetAPIKey(false)

// Little weird but windows returns a different error message than others
os := runtime.GOOS
switch os {
case "windows":
assert.ErrorContains(t, err, `config.toml: The system cannot find the file specified.`)
default:
assert.ErrorContains(t, err, `config.toml: no such file or directory`)
Copy link

Choose a reason for hiding this comment

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

Sometimes these error messages change or vary (e.g., different linux distros) and I'd like us to have a very obvious and actionable signal if/when that happens, since we're effectively "scraping" a non-guaranteed API.

Can we have an assertion somewhere that alerts us if neither of the two cases we know about today are met?

}

assert.Equal(t, "", key)

// In info mode, it should give a cleaner error about the key not being
// configured
logrus.SetLevel(logrus.InfoLevel)
key, err = c.Profile.GetAPIKey(false)
assert.EqualError(t, err, "you have not configured API keys yet")
assert.Equal(t, "", key)
}

func helperLoadBytes(t *testing.T, name string) []byte {
bytes, err := ioutil.ReadFile(name)
if err != nil {
Expand Down