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

refactor: modify enr error message #493

Merged
merged 5 commits into from
May 6, 2022
Merged

refactor: modify enr error message #493

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
baa5226
modified enr error message
xenowits May 5, 2022
7ab9c8c
move error to a new string
xenowits May 5, 2022
dbe8806
Merge branch 'main' of https://github.com/ObolNetwork/charon into xen…
xenowits May 6, 2022
e6f299d
fix issue
xenowits May 6, 2022
b4b0e5c
add test and refactor error kind
xenowits May 6, 2022
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
3 changes: 2 additions & 1 deletion cmd/enr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ func newEnrCmd(runFunc func(io.Writer, p2p.Config, string) error) *cobra.Command
// runNewENR loads the p2pkey from disk and prints the ENR for the provided config.
func runNewENR(w io.Writer, config p2p.Config, dataDir string) error {
key, err := p2p.LoadPrivKey(dataDir)
helpMsg := fmt.Sprintf("No ENR private key found in %s. If this is your first time running this client, create one with `charon create enr`.", p2p.KeyPath(dataDir))
if err != nil {
return err
return errors.New(helpMsg)
Copy link
Contributor

@corverroos corverroos May 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

You are assuming that this is the only error that can occur, which is not true. Also please use structured errors always.

Suggest

key, err := p2p.LoadP2PKey(dir)
if os.IsNotExist(err) {
  return errors.New("ENR private key not found. Maybe create one with `charon create enr`", z.Str("path", p2p.KeyPath(dataDir))
} else if err != nil {
  return err
}

Also please add a test for this!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For os.IsNotExist(err) to work, the error coming from calling p2p.LoadPrivKey should not be wrapped. wrapping changes the error type from *fs.PathError to structured.

But, when i attempt to simply return err, like,

func LoadPrivKey(dataDir string) (*ecdsa.PrivateKey, error) {
	key, err := crypto.LoadECDSA(KeyPath(dataDir))
	if err != nil {
		// return nil, errors.Wrap(err, "load key")
                // But, this throws an error by golangci-lint's wrapcheck linter
                	return nil, err
	}

	return key, nil
}

}

localEnode, db, err := p2p.NewLocalEnode(config, key)
Expand Down