Refactor error handling (#308)

use fmt instead of log

log.Fatal -> return err

set non-zero exit code on error

print to default err log

cleanup

fix vet

Co-authored-by: Norwin Roosen <git@nroo.de>
Co-authored-by: 6543 <6543@obermui.de>
Reviewed-on: https://gitea.com/gitea/tea/pulls/308
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
Reviewed-by: 6543 <6543@obermui.de>
Co-Authored-By: Norwin <noerw@noreply.gitea.io>
Co-Committed-By: Norwin <noerw@noreply.gitea.io>

cmd/issues/close.go

@@ -5,7 +5,7 @@
 package issues
 
 import (
-	"log"
+	"fmt"
 
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/context"
@@ -34,7 +34,7 @@ func editIssueState(cmd *cli.Context, opts gitea.EditIssueOption) error {
 	ctx := context.InitCommand(cmd)
 	ctx.Ensure(context.CtxRequirement{RemoteRepo: true})
 	if ctx.Args().Len() == 0 {
-		log.Fatal(ctx.Command.ArgsUsage)
+		return fmt.Errorf(ctx.Command.ArgsUsage)
 	}
 
 	index, err := utils.ArgToIndex(ctx.Args().First())

cmd/issues/list.go

@@ -5,8 +5,6 @@
 package issues
 
 import (
-	"log"
-
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/context"
 	"code.gitea.io/tea/modules/print"
@@ -47,7 +45,7 @@ func RunIssuesList(cmd *cli.Context) error {
 	})
 
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	print.IssuesList(issues, ctx.Output)

cmd/labels.go

@@ -5,7 +5,7 @@
 package cmd
 
 import (
-	"log"
+	"fmt"
 
 	"code.gitea.io/tea/cmd/labels"
 	"github.com/urfave/cli/v2"
@@ -34,6 +34,5 @@ func runLabels(ctx *cli.Context) error {
 }
 
 func runLabelsDetails(ctx *cli.Context) error {
-	log.Fatal("Not yet implemented.")
+	return fmt.Errorf("Not yet implemented")
-	return nil
 }

cmd/labels/create.go

@@ -80,11 +80,7 @@ func runLabelCreate(cmd *cli.Context) error {
 		}
 	}
 
-	if err != nil {
+	return err
-		log.Fatal(err)
-	}
-
-	return nil
 }
 
 func splitLabelLine(line string) (string, string, string) {

cmd/labels/delete.go

@@ -5,8 +5,6 @@
 package labels
 
 import (
-	"log"
-
 	"code.gitea.io/tea/modules/context"
 
 	"github.com/urfave/cli/v2"
@@ -31,9 +29,5 @@ func runLabelDelete(cmd *cli.Context) error {
 	ctx.Ensure(context.CtxRequirement{RemoteRepo: true})
 
 	_, err := ctx.Login.Client().DeleteLabel(ctx.Owner, ctx.Repo, ctx.Int64("id"))
-	if err != nil {
+	return err
-		log.Fatal(err)
-	}
-
-	return nil
 }

cmd/labels/list.go

@@ -5,8 +5,6 @@
 package labels
 
 import (
-	"log"
-
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/context"
 	"code.gitea.io/tea/modules/print"
@@ -44,7 +42,7 @@ func RunLabelsList(cmd *cli.Context) error {
 		ListOptions: ctx.GetListOptions(),
 	})
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	if ctx.IsSet("save") {

cmd/labels/update.go

@@ -5,8 +5,6 @@
 package labels
 
 import (
-	"log"
-
 	"code.gitea.io/tea/modules/context"
 
 	"code.gitea.io/sdk/gitea"
@@ -68,7 +66,7 @@ func runLabelUpdate(cmd *cli.Context) error {
 	})
 
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	return nil

cmd/login/list.go

@@ -5,8 +5,6 @@
 package login
 
 import (
-	"log"
-
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/config"
 	"code.gitea.io/tea/modules/print"
@@ -28,7 +26,7 @@ var CmdLoginList = cli.Command{
 func RunLoginList(cmd *cli.Context) error {
 	logins, err := config.GetLogins()
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 	print.LoginsList(logins, cmd.String("output"))
 	return nil

cmd/milestones/create.go

@@ -6,7 +6,6 @@ package milestones
 
 import (
 	"fmt"
-	"log"
 
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/context"
@@ -62,7 +61,7 @@ func runMilestonesCreate(cmd *cli.Context) error {
 		State:       state,
 	})
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	print.MilestoneDetails(mile)

cmd/milestones/list.go

@@ -5,8 +5,6 @@
 package milestones
 
 import (
-	"log"
-
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/context"
 	"code.gitea.io/tea/modules/print"
@@ -53,7 +51,7 @@ func RunMilestonesList(cmd *cli.Context) error {
 	})
 
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	print.MilestonesList(milestones, ctx.Output, state)

cmd/notifications.go

@@ -5,8 +5,6 @@
 package cmd
 
 import (
-	"log"
-
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/context"
 	"code.gitea.io/tea/modules/print"
@@ -76,7 +74,7 @@ func runNotifications(cmd *cli.Context) error {
 		})
 	}
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	print.NotificationsList(news, ctx.Output, ctx.Bool("all"))

cmd/open.go

@@ -5,7 +5,6 @@
 package cmd
 
 import (
-	"log"
 	"path"
 	"strings"
 
@@ -42,12 +41,11 @@ func runOpen(cmd *cli.Context) error {
 	case strings.EqualFold(number, "commits"):
 		repo, err := local_git.RepoForWorkdir()
 		if err != nil {
-			log.Fatal(err)
+			return err
 		}
 		b, err := repo.Head()
 		if err != nil {
-			log.Fatal(err)
+			return err
-			return nil
 		}
 		name := b.Name()
 		switch {
@@ -74,11 +72,6 @@ func runOpen(cmd *cli.Context) error {
 		suffix = number
 	}
 
-	u := path.Join(ctx.Login.URL, ctx.RepoSlug, suffix)
+	u := path.Join(ctx.Login.URL, ctx.Owner, ctx.Repo, suffix)
-	err := open.Run(u)
+	return open.Run(u)
-	if err != nil {
-		log.Fatal(err)
-	}
-
-	return nil
 }

cmd/organizations.go

@@ -5,7 +5,7 @@
 package cmd
 
 import (
-	"log"
+	"fmt"
 
 	"code.gitea.io/tea/cmd/organizations"
 
@@ -34,7 +34,5 @@ func runOrganizations(ctx *cli.Context) error {
 }
 
 func runOrganizationDetail(path string) error {
-
+	return fmt.Errorf("Not yet implemented")
-	log.Fatal("Not yet implemented.")
-	return nil
 }

cmd/organizations/delete.go

@@ -5,7 +5,7 @@
 package organizations
 
 import (
-	"log"
+	"fmt"
 
 	"code.gitea.io/tea/modules/context"
 	"github.com/urfave/cli/v2"
@@ -28,14 +28,12 @@ func RunOrganizationDelete(cmd *cli.Context) error {
 	client := ctx.Login.Client()
 
 	if ctx.Args().Len() < 1 {
-		log.Fatal("You have to specify the organization name you want to delete.")
+		return fmt.Errorf("You have to specify the organization name you want to delete")
-		return nil
 	}
 
 	response, err := client.DeleteOrg(ctx.Args().First())
 	if response != nil && response.StatusCode == 404 {
-		log.Fatal("The given organization does not exist.")
+		return fmt.Errorf("The given organization does not exist")
-		return nil
 	}
 
 	return err

cmd/organizations/list.go

@@ -5,8 +5,6 @@
 package organizations
 
 import (
-	"log"
-
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/context"
 	"code.gitea.io/tea/modules/print"
@@ -37,7 +35,7 @@ func RunOrganizationList(cmd *cli.Context) error {
 		ListOptions: ctx.GetListOptions(),
 	})
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	print.OrganizationsList(userOrganizations, ctx.Output)

cmd/pulls/checkout.go

@@ -5,7 +5,7 @@
 package pulls
 
 import (
-	"log"
+	"fmt"
 
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/context"
@@ -30,7 +30,7 @@ func runPullsCheckout(cmd *cli.Context) error {
 	ctx := context.InitCommand(cmd)
 	ctx.Ensure(context.CtxRequirement{LocalRepo: true})
 	if ctx.Args().Len() != 1 {
-		log.Fatal("Must specify a PR index")
+		return fmt.Errorf("Must specify a PR index")
 	}
 	idx, err := utils.ArgToIndex(ctx.Args().First())
 	if err != nil {

cmd/pulls/list.go

@@ -5,8 +5,6 @@
 package pulls
 
 import (
-	"log"
-
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/context"
 	"code.gitea.io/tea/modules/print"
@@ -45,7 +43,7 @@ func RunPullsList(cmd *cli.Context) error {
 	})
 
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	print.PullsList(prs, ctx.Output)

cmd/releases/create.go

@@ -6,7 +6,6 @@ package releases
 
 import (
 	"fmt"
-	"log"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -76,24 +75,23 @@ func runReleaseCreate(cmd *cli.Context) error {
 
 	if err != nil {
 		if resp != nil && resp.StatusCode == http.StatusConflict {
-			fmt.Println("error: There already is a release for this tag")
+			return fmt.Errorf("There already is a release for this tag")
-			return nil
 		}
-		log.Fatal(err)
+		return err
 	}
 
 	for _, asset := range ctx.StringSlice("asset") {
 		var file *os.File
 
 		if file, err = os.Open(asset); err != nil {
-			log.Fatal(err)
+			return err
 		}
 
 		filePath := filepath.Base(asset)
 
 		if _, _, err = ctx.Login.Client().CreateReleaseAttachment(ctx.Owner, ctx.Repo, release.ID, file, filePath); err != nil {
 			file.Close()
-			log.Fatal(err)
+			return err
 		}
 
 		file.Close()

cmd/releases/list.go

@@ -6,7 +6,6 @@ package releases
 
 import (
 	"fmt"
-	"log"
 
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/context"
@@ -38,7 +37,7 @@ func RunReleasesList(cmd *cli.Context) error {
 		ListOptions: ctx.GetListOptions(),
 	})
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	print.ReleasesList(releases, ctx.Output)

cmd/repos/search.go

@@ -5,7 +5,7 @@
 package repos
 
 import (
-	"log"
+	"fmt"
 	"strings"
 
 	"code.gitea.io/tea/cmd/flags"
@@ -67,7 +67,7 @@ func runReposSearch(cmd *cli.Context) error {
 		if err != nil {
 			// HACK: the client does not return a response on 404, so we can't check res.StatusCode
 			if err.Error() != "404 Not Found" {
-				log.Fatal("could not find owner: ", err)
+				return fmt.Errorf("Could not find owner: %s", err)
 			}
 
 			// if owner is no org, its a user

cmd/times/add.go

@@ -6,7 +6,6 @@ package times
 
 import (
 	"fmt"
-	"log"
 	"strings"
 	"time"
 
@@ -41,20 +40,16 @@ func runTrackedTimesAdd(cmd *cli.Context) error {
 
 	issue, err := utils.ArgToIndex(ctx.Args().First())
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	duration, err := time.ParseDuration(strings.Join(ctx.Args().Tail(), ""))
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	_, _, err = ctx.Login.Client().AddTime(ctx.Owner, ctx.Repo, issue, gitea.AddTimeOption{
 		Time: int64(duration.Seconds()),
 	})
-	if err != nil {
+	return err
-		log.Fatal(err)
-	}
-
-	return nil
 }

cmd/times/delete.go

@@ -6,7 +6,6 @@ package times
 
 import (
 	"fmt"
-	"log"
 	"strconv"
 
 	"code.gitea.io/tea/cmd/flags"
@@ -37,18 +36,14 @@ func runTrackedTimesDelete(cmd *cli.Context) error {
 
 	issue, err := utils.ArgToIndex(ctx.Args().First())
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	timeID, err := strconv.ParseInt(ctx.Args().Get(1), 10, 64)
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	_, err = client.DeleteTime(ctx.Owner, ctx.Repo, issue, timeID)
-	if err != nil {
+	return err
-		log.Fatal(err)
-	}
-
-	return nil
 }

cmd/times/reset.go

@@ -6,7 +6,6 @@ package times
 
 import (
 	"fmt"
-	"log"
 
 	"code.gitea.io/tea/cmd/flags"
 	"code.gitea.io/tea/modules/context"
@@ -36,13 +35,9 @@ func runTrackedTimesReset(cmd *cli.Context) error {
 
 	issue, err := utils.ArgToIndex(ctx.Args().First())
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	_, err = client.ResetIssueTime(ctx.Owner, ctx.Repo, issue)
-	if err != nil {
+	return err
-		log.Fatal(err)
-	}
-
-	return nil
 }

main.go

@@ -1,4 +1,4 @@
-// Copyright 2018 The Gitea Authors. All rights reserved.
+// Copyright 2020 The Gitea Authors. All rights reserved.
 // Use of this source code is governed by a MIT-style
 // license that can be found in the LICENSE file.
 
@@ -6,7 +6,7 @@
 package main // import "code.gitea.io/tea"
 
 import (
-	"log"
+	"fmt"
 	"os"
 	"strings"
 
@@ -25,7 +25,6 @@ func main() {
 	app := cli.NewApp()
 	app.Name = "tea"
 	app.Usage = "Command line tool to interact with Gitea"
-	app.Description = ``
 	app.Version = Version + formatBuiltWith(Tags)
 	app.Commands = []*cli.Command{
 		&cmd.CmdLogin,
@@ -45,7 +44,10 @@ func main() {
 	app.EnableBashCompletion = true
 	err := app.Run(os.Args)
 	if err != nil {
-		log.Fatalf("Failed to run app with %s: %v", os.Args, err)
+		// app.Run already exits for errors implementing ErrorCoder,
+		// so we only handle generic errors with code 1 here.
+		fmt.Fprintf(app.ErrWriter, "Error: %v\n", err)
+		os.Exit(1)
 	}
 }
 

modules/task/issue_create.go

@@ -6,7 +6,6 @@ package task
 
 import (
 	"fmt"
-	"log"
 
 	"code.gitea.io/sdk/gitea"
 	"code.gitea.io/tea/modules/config"
@@ -34,12 +33,12 @@ func CreateIssue(login *config.Login, repoOwner, repoName, title, description st
 	})
 
 	if err != nil {
-		log.Fatalf("could not create issue: %s", err)
+		return fmt.Errorf("could not create issue: %s", err)
 	}
 
 	print.IssueDetails(issue)
 
 	fmt.Println(issue.HTMLURL)
 
-	return err
+	return nil
 }

modules/task/labels_export.go

@@ -6,7 +6,6 @@ package task
 
 import (
 	"fmt"
-	"log"
 	"os"
 
 	"code.gitea.io/sdk/gitea"
@@ -16,7 +15,7 @@ import (
 func LabelsExport(labels []*gitea.Label, path string) error {
 	f, err := os.Create(path)
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 	defer f.Close()
 

modules/task/login_create.go

@@ -6,7 +6,6 @@ package task
 
 import (
 	"fmt"
-	"log"
 	"os"
 	"time"
 
@@ -21,7 +20,7 @@ func CreateLogin(name, token, user, passwd, sshKey, giteaURL string, insecure bo
 	// checks ...
 	// ... if we have a url
 	if len(giteaURL) == 0 {
-		log.Fatal("You have to input Gitea server URL")
+		return fmt.Errorf("You have to input Gitea server URL")
 	}
 
 	// ... if there already exist a login with same name
@@ -35,17 +34,17 @@ func CreateLogin(name, token, user, passwd, sshKey, giteaURL string, insecure bo
 
 	// .. if we have enough information to authenticate
 	if len(token) == 0 && (len(user)+len(passwd)) == 0 {
-		log.Fatal("No token set")
+		return fmt.Errorf("No token set")
 	} else if len(user) != 0 && len(passwd) == 0 {
-		log.Fatal("No password set")
+		return fmt.Errorf("No password set")
 	} else if len(user) == 0 && len(passwd) != 0 {
-		log.Fatal("No user set")
+		return fmt.Errorf("No user set")
 	}
 
 	// Normalize URL
 	serverURL, err := utils.NormalizeURL(giteaURL)
 	if err != nil {
-		log.Fatal("Unable to parse URL", err)
+		return fmt.Errorf("Unable to parse URL: %s", err)
 	}
 
 	login := config.Login{
@@ -60,23 +59,21 @@ func CreateLogin(name, token, user, passwd, sshKey, giteaURL string, insecure bo
 	client := login.Client()
 
 	if len(token) == 0 {
-		login.Token, err = generateToken(client, user, passwd)
+		if login.Token, err = generateToken(client, user, passwd); err != nil {
-		if err != nil {
+			return err
-			log.Fatal(err)
 		}
 	}
 
 	// Verify if authentication works and get user info
 	u, _, err := client.GetMyUserInfo()
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 	login.User = u.UserName
 
 	if len(login.Name) == 0 {
-		login.Name, err = GenerateLoginName(giteaURL, login.User)
+		if login.Name, err = GenerateLoginName(giteaURL, login.User); err != nil {
-		if err != nil {
+			return err
-			log.Fatal(err)
 		}
 	}
 
@@ -91,9 +88,8 @@ func CreateLogin(name, token, user, passwd, sshKey, giteaURL string, insecure bo
 		}
 	}
 
-	err = config.AddLogin(&login)
+	if err = config.AddLogin(&login); err != nil {
-	if err != nil {
+		return err
-		log.Fatal(err)
 	}
 
 	fmt.Printf("Login as %s on %s successful. Added this login as %s\n", login.User, login.URL, login.Name)

modules/task/pull_create.go

@@ -6,7 +6,6 @@ package task
 
 import (
 	"fmt"
-	"log"
 	"strings"
 
 	"code.gitea.io/sdk/gitea"
@@ -24,14 +23,14 @@ func CreatePull(login *config.Login, repoOwner, repoName, base, head, title, des
 	// open local git repo
 	localRepo, err := local_git.RepoForWorkdir()
 	if err != nil {
-		log.Fatal("could not open local repo: ", err)
+		return fmt.Errorf("Could not open local repo: %s", err)
 	}
 
 	// push if possible
-	log.Println("git push")
+	fmt.Println("git push")
 	err = localRepo.Push(&git.PushOptions{})
 	if err != nil && err != git.NoErrAlreadyUpToDate {
-		log.Printf("Error occurred during 'git push':\n%s\n", err.Error())
+		fmt.Printf("Error occurred during 'git push':\n%s\n", err.Error())
 	}
 
 	// default is default branch
@@ -74,7 +73,7 @@ func CreatePull(login *config.Login, repoOwner, repoName, base, head, title, des
 	})
 
 	if err != nil {
-		log.Fatalf("could not create PR from %s to %s:%s: %s", head, repoOwner, base, err)
+		return fmt.Errorf("Could not create PR from %s to %s:%s: %s", head, repoOwner, base, err)
 	}
 
 	print.PullDetails(pr, nil)