[ioctl] fix Errors unhandled (#3567)

* fix error unhandle * fix pkg error unhandled * fix error unhandled in tools * fix compress close handle * return error for AskToConfirm Co-authored-by: Raullen Chai <raullenchai@gmail.com>
iotexproject · Jul 21, 2022 · bc03a0e · bc03a0e
1 parent 7cfd272
commit bc03a0e
Show file tree

Hide file tree

Showing 25 changed files with 107 additions and 51 deletions.
diff --git a/ioctl/client.go b/ioctl/client.go
@@ -53,7 +53,7 @@ type (
 		// SelectTranslation select a translation based on UILanguage
 		SelectTranslation(map[config.Language]string) (string, config.Language)
 		// AskToConfirm asks user to confirm from terminal, true to continue
-		AskToConfirm(string) bool
+		AskToConfirm(string) (bool, error)
 		// ReadSecret reads password from terminal
 		ReadSecret() (string, error)
 		// Execute a bash command
@@ -173,12 +173,14 @@ func (c *client) SetInsecureWithFlag(cb func(*bool, string, bool, string)) {
 	cb(&c.insecure, "insecure", !c.cfg.SecureConnect, usage)
 }
 
-func (c *client) AskToConfirm(info string) bool {
+func (c *client) AskToConfirm(info string) (bool, error) {
 	message := ConfirmationMessage{Info: info, Options: []string{"yes"}}
 	fmt.Println(message.String())
 	var confirm string
-	fmt.Scanf("%s", &confirm)
-	return strings.EqualFold(confirm, "yes")
+	if _, err := fmt.Scanf("%s", &confirm); err != nil {
+		return false, err
+	}
+	return strings.EqualFold(confirm, "yes"), nil
 }
 
 func (c *client) SelectTranslation(trls map[config.Language]string) (string, config.Language) {

diff --git a/ioctl/client_test.go b/ioctl/client_test.go
@@ -38,9 +38,10 @@ func TestAskToConfirm(t *testing.T) {
 	r := require.New(t)
 	c := NewClient(config.Config{}, "")
 	defer c.Stop(context.Background())
-	blang := c.AskToConfirm("test")
+	confirmed, err := c.AskToConfirm("test")
 	// no input
-	r.False(blang)
+	r.Equal("EOF", err.Error())
+	r.False(confirmed)
 }
 
 func TestAPIServiceClient(t *testing.T) {

diff --git a/ioctl/cmd/account/accountcreateadd.go b/ioctl/cmd/account/accountcreateadd.go
@@ -56,7 +56,9 @@ func accountCreateAdd(args []string) error {
 			"but bind the alias to the new one.\nWould you like to continue?\n", alias, addr)
 		message := output.ConfirmationMessage{Info: info, Options: []string{"yes"}}
 		fmt.Println(message.String())
-		fmt.Scanf("%s", &confirm)
+		if _, err := fmt.Scanf("%s", &confirm); err != nil {
+			return output.NewError(output.InputError, "failed to input yes", err)
+		}
 		if !strings.EqualFold(confirm, "yes") {
 			output.PrintResult("quit")
 			return nil

diff --git a/ioctl/cmd/account/accountdelete.go b/ioctl/cmd/account/accountdelete.go
@@ -91,7 +91,9 @@ func accountDelete(arg string) error {
 		"Type 'YES' to continue, quit for anything else.")
 	message := output.ConfirmationMessage{Info: info, Options: []string{"yes"}}
 	fmt.Println(message.String())
-	fmt.Scanf("%s", &confirm)
+	if _, err := fmt.Scanf("%s", &confirm); err != nil {
+		return output.NewError(output.InputError, "failed to input yes", err)
+	}
 	if !strings.EqualFold(confirm, "yes") {
 		output.PrintResult("quit")
 		return nil

diff --git a/ioctl/cmd/action/action.go b/ioctl/cmd/action/action.go
@@ -295,7 +295,9 @@ func SendAction(elp action.Envelope, signer string) error {
 		message := output.ConfirmationMessage{Info: info, Options: []string{"yes"}}
 		fmt.Println(message.String())
 
-		fmt.Scanf("%s", &confirm)
+		if _, err := fmt.Scanf("%s", &confirm); err != nil {
+			return output.NewError(output.InputError, "failed to input yes", err)
+		}
 		if !strings.EqualFold(confirm, "yes") {
 			output.PrintResult("quit")
 			return nil

diff --git a/ioctl/cmd/action/xrc20.go b/ioctl/cmd/action/xrc20.go
@@ -83,7 +83,9 @@ func init() {
 		config.ReadConfig.Endpoint, config.TranslateInLang(_flagXrc20EndPointUsages, config.UILanguage))
 	Xrc20Cmd.PersistentFlags().BoolVar(&config.Insecure, "insecure", config.Insecure,
 		config.TranslateInLang(_flagXrc20InsecureUsages, config.UILanguage))
-	cobra.MarkFlagRequired(Xrc20Cmd.PersistentFlags(), "contract-address")
+	if err := cobra.MarkFlagRequired(Xrc20Cmd.PersistentFlags(), "contract-address"); err != nil {
+		fmt.Printf("failed to mark flag: %v\n", err)
+	}
 }
 
 func parseAmount(contract address.Address, amount string) (*big.Int, error) {

diff --git a/ioctl/cmd/contract/contractshare.go b/ioctl/cmd/contract/contractshare.go
@@ -102,7 +102,9 @@ func isReadOnly(path string) bool {
 		}
 		readOnly = true
 	}
-	file.Close()
+	if err = file.Close(); err != nil {
+		log.Printf("fialed to close file: %v", err)
+	}
 	return readOnly
 }
 
@@ -140,7 +142,7 @@ func share(args []string) error {
 		return output.NewError(output.FlagError, "failed to get IoTeX ide url instance", nil)
 	}
 
-	filepath.Walk(_givenPath, func(path string, info os.FileInfo, err error) error {
+	if err := filepath.Walk(_givenPath, func(path string, info os.FileInfo, err error) error {
 		if !isDir(path) {
 			relPath, err := filepath.Rel(_givenPath, path)
 			if err != nil {
@@ -152,7 +154,9 @@ func share(args []string) error {
 			}
 		}
 		return nil
-	})
+	}); err != nil {
+		return output.NewError(output.ReadFileError, "failed to walk directory", err)
+	}
 
 	log.Printf("Listening on 127.0.0.1:65520, Please open your IDE ( %s ) to connect to local files", _iotexIDE)
 

diff --git a/ioctl/cmd/hdwallet/hdwalletdelete.go b/ioctl/cmd/hdwallet/hdwalletdelete.go
@@ -44,7 +44,9 @@ func hdwalletDelete() error {
 		"Type 'YES' to continue, quit for anything else.")
 	message := output.ConfirmationMessage{Info: info, Options: []string{"yes"}}
 	fmt.Println(message.String())
-	fmt.Scanf("%s", &confirm)
+	if _, err := fmt.Scanf("%s", &confirm); err != nil {
+		return output.NewError(output.InputError, "failed to input yes", err)
+	}
 	if !strings.EqualFold(confirm, "yes") {
 		output.PrintResult("quit")
 		return nil

diff --git a/ioctl/cmd/node/nodedelegate.go b/ioctl/cmd/node/nodedelegate.go
@@ -253,7 +253,9 @@ func delegatesV2(pb *vote.ProbationList, epochMeta *iotexapi.GetEpochMetaRespons
 			ProbatedStatus: isProbated,
 		})
 	}
-	fillMessage(cli, message, aliases, isActive, pb)
+	if err = fillMessage(cli, message, aliases, isActive, pb); err != nil {
+		return err
+	}
 	return sortAndPrint(message)
 }
 

diff --git a/ioctl/config/configsetget.go b/ioctl/config/configsetget.go
@@ -239,7 +239,9 @@ func set(args []string) error {
 		case args[1] == "custom":
 			output.PrintQuery(`Please enter a custom link below:("Example: iotexscan.io/action/")`)
 			var link string
-			fmt.Scanln(&link)
+			if _, err := fmt.Scanln(&link); err != nil {
+				return output.NewError(output.InputError, "failed to input link", err)
+			}
 			match, err := regexp.MatchString(_urlPattern, link)
 			if err != nil {
 				return output.NewError(output.UndefinedError, "failed to validate link", nil)

diff --git a/ioctl/flag/flag.go b/ioctl/flag/flag.go
@@ -7,6 +7,8 @@
 package flag
 
 import (
+	"fmt"
+
 	"github.com/spf13/cobra"
 
 	"github.com/iotexproject/iotex-core/ioctl/config"
@@ -57,7 +59,9 @@ type (
 )
 
 func (f *flagBase) MarkFlagRequired(cmd *cobra.Command) {
-	cmd.MarkFlagRequired(f.label)
+	if err := cmd.MarkFlagRequired(f.label); err != nil {
+		fmt.Printf("failed to mark flag %s: %v\n", f.label, err)
+	}
 }
 
 func (f *flagBase) Label() string {

diff --git a/ioctl/newcmd/account/accountcreateadd.go b/ioctl/newcmd/account/accountcreateadd.go
@@ -71,7 +71,11 @@ func NewAccountCreateAdd(client ioctl.Client) *cobra.Command {
 			}
 
 			if addr, ok := client.Config().Aliases[args[0]]; ok {
-				if !client.AskToConfirm(fmt.Sprintf(_aliasHasAlreadyUsed, args[0], addr)) {
+				confirmed, err := client.AskToConfirm(fmt.Sprintf(_aliasHasAlreadyUsed, args[0], addr))
+				if err != nil {
+					return errors.Wrap(err, "failed to ask confirm")
+				}
+				if !confirmed {
 					cmd.Println(infoQuit)
 					return nil
 				}

diff --git a/ioctl/newcmd/account/accountcreateadd_test.go b/ioctl/newcmd/account/accountcreateadd_test.go
@@ -29,7 +29,7 @@ func TestNewAccountCreateAdd(t *testing.T) {
 	require.NoError(err)
 
 	client.EXPECT().ReadSecret().Return(pwd, nil).Times(4)
-	client.EXPECT().AskToConfirm(gomock.Any()).Return(true).Times(2)
+	client.EXPECT().AskToConfirm(gomock.Any()).Return(true, nil).Times(2)
 	client.EXPECT().Config().Return(config.Config{
 		Wallet: testWallet,
 		Aliases: map[string]string{
@@ -57,7 +57,7 @@ func TestNewAccountCreateAdd(t *testing.T) {
 	})
 
 	t.Run("failed to confirm", func(t *testing.T) {
-		client.EXPECT().AskToConfirm(gomock.Any()).Return(false)
+		client.EXPECT().AskToConfirm(gomock.Any()).Return(false, nil)
 
 		cmd := NewAccountCreateAdd(client)
 		_, err := util.ExecuteCmd(cmd, "aaa")

diff --git a/ioctl/newcmd/account/accountdelete.go b/ioctl/newcmd/account/accountdelete.go
@@ -120,14 +120,18 @@ func NewAccountDelete(client ioctl.Client) *cobra.Command {
 			if _, err = os.Stat(filePath); err != nil {
 				return errors.Wrapf(err, _failToFindAccount, addr)
 			}
-			if !client.AskToConfirm(_infoWarn) {
+			confirmed, err := client.AskToConfirm(_infoWarn)
+			if err != nil {
+				return errors.Wrap(err, "failed to ask confirm")
+			}
+			if !confirmed {
 				cmd.Println(_infoQuit)
 				return nil
 			}
-			if err := os.Remove(filePath); err != nil {
+			if err = os.Remove(filePath); err != nil {
 				return errors.Wrap(err, _failToRemoveKeystoreFile)
 			}
-			if err := client.DeleteAlias(client.AliasMap()[addr]); err != nil {
+			if err = client.DeleteAlias(client.AliasMap()[addr]); err != nil {
 				return errors.Wrap(err, _failToWriteToConfigFile)
 			}
 			cmd.Println(fmt.Sprintf(_resultSuccess, addr))

diff --git a/ioctl/newcmd/account/accountdelete_test.go b/ioctl/newcmd/account/accountdelete_test.go
@@ -44,12 +44,12 @@ func TestNewAccountDelete(t *testing.T) {
 			"io1uwnr55vqmhf3xeg5phgurlyl702af6eju542s1": "ccc",
 		})
 
-		client.EXPECT().AskToConfirm(gomock.Any()).Return(false)
+		client.EXPECT().AskToConfirm(gomock.Any()).Return(false, nil)
 		cmd := NewAccountDelete(client)
 		_, err := util.ExecuteCmd(cmd)
 		require.NoError(err)
 
-		client.EXPECT().AskToConfirm(gomock.Any()).Return(true)
+		client.EXPECT().AskToConfirm(gomock.Any()).Return(true, nil)
 		client.EXPECT().DeleteAlias("aaa").Return(nil)
 		cmd = NewAccountDelete(client)
 		result, err := util.ExecuteCmd(cmd)
@@ -82,7 +82,7 @@ func TestNewAccountDelete(t *testing.T) {
 		crypto.WritePrivateKeyToPem(pemFilePath, priKey2.(*crypto.P256sm2PrvKey), "test")
 		client.EXPECT().AddressWithDefaultIfNotExist(gomock.Any()).Return(addr2.String(), nil)
 
-		client.EXPECT().AskToConfirm(gomock.Any()).Return(true)
+		client.EXPECT().AskToConfirm(gomock.Any()).Return(true, nil)
 		client.EXPECT().DeleteAlias("aaa").Return(nil)
 		cmd := NewAccountDelete(client)
 		result, err := util.ExecuteCmd(cmd)

diff --git a/ioctl/newcmd/action/action.go b/ioctl/newcmd/action/action.go
@@ -396,10 +396,14 @@ func SendAction(client ioctl.Client, cmd *cobra.Command, elp action.Envelope, si
 	if !getAssumeYesFlagValue(cmd) {
 		infoWarn := selectTranslation(client, _infoWarn)
 		infoQuit := selectTranslation(client, _infoQuit)
-		if !client.AskToConfirm(infoWarn) {
+		confirmed, err := client.AskToConfirm(infoWarn)
+		if err != nil {
+			return errors.Wrap(err, "failed to ask confirm")
+		}
+		if !confirmed {
 			cmd.Println(infoQuit)
+			return nil
 		}
-		return nil
 	}
 
 	return SendRaw(client, cmd, selp)

diff --git a/ioctl/newcmd/hdwallet/hdwalletdelete.go b/ioctl/newcmd/hdwallet/hdwalletdelete.go
@@ -9,6 +9,7 @@ package hdwallet
 import (
 	"fmt"
 
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 
 	"github.com/iotexproject/iotex-core/ioctl"
@@ -36,7 +37,11 @@ func NewHdwalletDeleteCmd(client ioctl.Client) *cobra.Command {
 			info := fmt.Sprintf("** This is an irreversible action!\n" +
 				"Once an hdwallet is deleted, all the assets under this hdwallet may be lost!\n" +
 				"Type 'YES' to continue, quit for anything else.")
-			if !client.AskToConfirm(info) {
+			confirmed, err := client.AskToConfirm(info)
+			if err != nil {
+				return errors.Wrap(err, "failed to ask confirm")
+			}
+			if !confirmed {
 				cmd.Println("quit")
 				return nil
 			}

diff --git a/ioctl/newcmd/hdwallet/hdwalletdelete_test.go b/ioctl/newcmd/hdwallet/hdwalletdelete_test.go
@@ -25,7 +25,7 @@ func TestNewHdwalletDeleteCmd(t *testing.T) {
 	client.EXPECT().SelectTranslation(gomock.Any()).Return("mockTranslationString", config.English).Times(4)
 
 	t.Run("delete hdwallet", func(t *testing.T) {
-		client.EXPECT().AskToConfirm(gomock.Any()).Return(true)
+		client.EXPECT().AskToConfirm(gomock.Any()).Return(true, nil)
 		client.EXPECT().RemoveHdWalletConfigFile().Return(nil)
 
 		cmd := NewHdwalletDeleteCmd(client)
@@ -35,7 +35,7 @@ func TestNewHdwalletDeleteCmd(t *testing.T) {
 	})
 
 	t.Run("quit hdwallet delete command", func(t *testing.T) {
-		client.EXPECT().AskToConfirm(gomock.Any()).Return(false)
+		client.EXPECT().AskToConfirm(gomock.Any()).Return(false, nil)
 
 		cmd := NewHdwalletDeleteCmd(client)
 		result, err := util.ExecuteCmd(cmd)

diff --git a/ioctl/newcmd/update/update.go b/ioctl/newcmd/update/update.go
@@ -86,13 +86,17 @@ func NewUpdateCmd(c ioctl.Client) *cobra.Command {
 				return errors.New(fmt.Sprintf(invalidVersionType, versionType))
 			}
 
-			if !c.AskToConfirm(_infoWarn) {
+			confirmed, err := c.AskToConfirm(_infoWarn)
+			if err != nil {
+				return errors.Wrap(err, "failed to ask confirm")
+			}
+			if !confirmed {
 				cmd.Println(_infoQuit)
 				return nil
 			}
 			cmd.Printf(info, versionType)
 
-			if err := c.Execute(cmdString); err != nil {
+			if err = c.Execute(cmdString); err != nil {
 				return errors.Wrap(err, fail)
 			}
 			cmd.Println(success)

diff --git a/ioctl/newcmd/update/update_test.go b/ioctl/newcmd/update/update_test.go
@@ -26,7 +26,7 @@ func TestNewUpdateCmd(t *testing.T) {
 	expectedValue := "ioctl is up-to-date now."
 	client.EXPECT().SelectTranslation(gomock.Any()).Return(expectedValue,
 		config.English).Times(18)
-	client.EXPECT().AskToConfirm(gomock.Any()).Return(true).Times(2)
+	client.EXPECT().AskToConfirm(gomock.Any()).Return(true, nil).Times(2)
 	client.EXPECT().Execute(gomock.Any()).Return(nil).Times(2)
 
 	t.Run("update cli with stable", func(t *testing.T) {
@@ -47,7 +47,7 @@ func TestNewUpdateCmd(t *testing.T) {
 		expectedError := errors.New("failed to execute bash command")
 		client.EXPECT().SelectTranslation(gomock.Any()).Return("mockTranslationResult",
 			config.English).Times(9)
-		client.EXPECT().AskToConfirm(gomock.Any()).Return(true).Times(1)
+		client.EXPECT().AskToConfirm(gomock.Any()).Return(true, nil).Times(1)
 		client.EXPECT().Execute(gomock.Any()).Return(expectedError).Times(1)
 
 		cmd := NewUpdateCmd(client)

diff --git a/pkg/compress/compress.go b/pkg/compress/compress.go
@@ -61,12 +61,13 @@ func CompGzip(data []byte) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	_, err = w.Write(data)
-	if err != nil {
-		w.Close()
+	if _, err = w.Write(data); err != nil {
+		err = w.Close()
+		return nil, err
+	}
+	if err = w.Close(); err != nil {
 		return nil, err
 	}
-	w.Close()
 	output := bb.Bytes()
 	return output, nil
 }
@@ -77,7 +78,9 @@ func DecompGzip(data []byte) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	r.Close()
+	if err = r.Close(); err != nil {
+		return nil, err
+	}
 	return io.ReadAll(r)
 }
 

diff --git a/state/account.go b/state/account.go
@@ -102,7 +102,7 @@ func (st *Account) FromProto(acPb *accountpb.Account) {
 	} else {
 		balance, ok := new(big.Int).SetString(acPb.Balance, 10)
 		if !ok {
-			errors.Errorf("invalid balance %s", acPb.Balance)
+			panic(errors.Errorf("invalid balance %s", acPb.Balance))
 		}
 		st.Balance = balance
 	}

diff --git a/test/mock/mock_ioctlclient/mock_ioctlclient.go b/test/mock/mock_ioctlclient/mock_ioctlclient.go