From a948fd7e10efbf50f6060c9476243cda651f491b Mon Sep 17 00:00:00 2001 From: Norwin Date: Thu, 17 Dec 2020 00:18:10 +0800 Subject: [PATCH] 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 Co-authored-by: 6543 <6543@obermui.de> Reviewed-on: https://gitea.com/gitea/tea/pulls/308 Reviewed-by: Lunny Xiao Reviewed-by: 6543 <6543@obermui.de> Co-Authored-By: Norwin Co-Committed-By: Norwin --- cmd/issues/close.go | 4 ++-- cmd/issues/list.go | 4 +--- cmd/labels.go | 5 ++--- cmd/labels/create.go | 6 +----- cmd/labels/delete.go | 8 +------- cmd/labels/list.go | 4 +--- cmd/labels/update.go | 4 +--- cmd/login/list.go | 4 +--- cmd/milestones/create.go | 3 +-- cmd/milestones/list.go | 4 +--- cmd/notifications.go | 4 +--- cmd/open.go | 15 ++++----------- cmd/organizations.go | 6 ++---- cmd/organizations/delete.go | 8 +++----- cmd/organizations/list.go | 4 +--- cmd/pulls/checkout.go | 4 ++-- cmd/pulls/list.go | 4 +--- cmd/releases/create.go | 10 ++++------ cmd/releases/list.go | 3 +-- cmd/repos/search.go | 4 ++-- cmd/times/add.go | 11 +++-------- cmd/times/delete.go | 11 +++-------- cmd/times/reset.go | 9 ++------- main.go | 10 ++++++---- modules/task/issue_create.go | 5 ++--- modules/task/labels_export.go | 3 +-- modules/task/login_create.go | 28 ++++++++++++---------------- modules/task/pull_create.go | 9 ++++----- 28 files changed, 66 insertions(+), 128 deletions(-) diff --git a/cmd/issues/close.go b/cmd/issues/close.go index 1d3c4d8..634ecd3 100644 --- a/cmd/issues/close.go +++ b/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()) diff --git a/cmd/issues/list.go b/cmd/issues/list.go index 87c3ea9..1a722fe 100644 --- a/cmd/issues/list.go +++ b/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) diff --git a/cmd/labels.go b/cmd/labels.go index ed953f8..722e1d0 100644 --- a/cmd/labels.go +++ b/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 nil + return fmt.Errorf("Not yet implemented") } diff --git a/cmd/labels/create.go b/cmd/labels/create.go index afc2856..4f9773d 100644 --- a/cmd/labels/create.go +++ b/cmd/labels/create.go @@ -80,11 +80,7 @@ func runLabelCreate(cmd *cli.Context) error { } } - if err != nil { - log.Fatal(err) - } - - return nil + return err } func splitLabelLine(line string) (string, string, string) { diff --git a/cmd/labels/delete.go b/cmd/labels/delete.go index 890d628..0e32e9d 100644 --- a/cmd/labels/delete.go +++ b/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 { - log.Fatal(err) - } - - return nil + return err } diff --git a/cmd/labels/list.go b/cmd/labels/list.go index 4d8ccda..f4b8c62 100644 --- a/cmd/labels/list.go +++ b/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") { diff --git a/cmd/labels/update.go b/cmd/labels/update.go index 41c430d..f3fb8b6 100644 --- a/cmd/labels/update.go +++ b/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 diff --git a/cmd/login/list.go b/cmd/login/list.go index 782c2a8..f30fec0 100644 --- a/cmd/login/list.go +++ b/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 diff --git a/cmd/milestones/create.go b/cmd/milestones/create.go index 4b1ff55..11e6bd7 100644 --- a/cmd/milestones/create.go +++ b/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) diff --git a/cmd/milestones/list.go b/cmd/milestones/list.go index 699a00a..9b168f6 100644 --- a/cmd/milestones/list.go +++ b/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) diff --git a/cmd/notifications.go b/cmd/notifications.go index 32cc42f..ad4f104 100644 --- a/cmd/notifications.go +++ b/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")) diff --git a/cmd/open.go b/cmd/open.go index 78fba30..31f8006 100644 --- a/cmd/open.go +++ b/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 nil + return err } 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) - err := open.Run(u) - if err != nil { - log.Fatal(err) - } - - return nil + u := path.Join(ctx.Login.URL, ctx.Owner, ctx.Repo, suffix) + return open.Run(u) } diff --git a/cmd/organizations.go b/cmd/organizations.go index fa51376..d849097 100644 --- a/cmd/organizations.go +++ b/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 { - - log.Fatal("Not yet implemented.") - return nil + return fmt.Errorf("Not yet implemented") } diff --git a/cmd/organizations/delete.go b/cmd/organizations/delete.go index 5f1f6b6..f194530 100644 --- a/cmd/organizations/delete.go +++ b/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 nil + return fmt.Errorf("You have to specify the organization name you want to delete") } response, err := client.DeleteOrg(ctx.Args().First()) if response != nil && response.StatusCode == 404 { - log.Fatal("The given organization does not exist.") - return nil + return fmt.Errorf("The given organization does not exist") } return err diff --git a/cmd/organizations/list.go b/cmd/organizations/list.go index aea12fd..6b63706 100644 --- a/cmd/organizations/list.go +++ b/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) diff --git a/cmd/pulls/checkout.go b/cmd/pulls/checkout.go index 981a2d4..4ca9bf9 100644 --- a/cmd/pulls/checkout.go +++ b/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 { diff --git a/cmd/pulls/list.go b/cmd/pulls/list.go index 2010fe7..55d4476 100644 --- a/cmd/pulls/list.go +++ b/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) diff --git a/cmd/releases/create.go b/cmd/releases/create.go index e9c68e0..f496cda 100644 --- a/cmd/releases/create.go +++ b/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 nil + return fmt.Errorf("There already is a release for this tag") } - 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() diff --git a/cmd/releases/list.go b/cmd/releases/list.go index d2fee37..3b43d48 100644 --- a/cmd/releases/list.go +++ b/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) diff --git a/cmd/repos/search.go b/cmd/repos/search.go index 6c48e4a..1d97ce5 100644 --- a/cmd/repos/search.go +++ b/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 diff --git a/cmd/times/add.go b/cmd/times/add.go index 2de9c49..928e65f 100644 --- a/cmd/times/add.go +++ b/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 { - log.Fatal(err) - } - - return nil + return err } diff --git a/cmd/times/delete.go b/cmd/times/delete.go index 91ed2c6..8051040 100644 --- a/cmd/times/delete.go +++ b/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 { - log.Fatal(err) - } - - return nil + return err } diff --git a/cmd/times/reset.go b/cmd/times/reset.go index 382af56..2be4d9e 100644 --- a/cmd/times/reset.go +++ b/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 { - log.Fatal(err) - } - - return nil + return err } diff --git a/main.go b/main.go index a4b82af..c9e9c6f 100644 --- a/main.go +++ b/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) } } diff --git a/modules/task/issue_create.go b/modules/task/issue_create.go index 3808a9f..5a260cf 100644 --- a/modules/task/issue_create.go +++ b/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 } diff --git a/modules/task/labels_export.go b/modules/task/labels_export.go index a41a98a..bb8a801 100644 --- a/modules/task/labels_export.go +++ b/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() diff --git a/modules/task/login_create.go b/modules/task/login_create.go index bb3ccaa..b0901c5 100644 --- a/modules/task/login_create.go +++ b/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 err != nil { - log.Fatal(err) + if login.Token, err = generateToken(client, user, passwd); err != nil { + return 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 err != nil { - log.Fatal(err) + if login.Name, err = GenerateLoginName(giteaURL, login.User); err != nil { + return err } } @@ -91,9 +88,8 @@ func CreateLogin(name, token, user, passwd, sshKey, giteaURL string, insecure bo } } - err = config.AddLogin(&login) - if err != nil { - log.Fatal(err) + if err = config.AddLogin(&login); err != nil { + return err } fmt.Printf("Login as %s on %s successful. Added this login as %s\n", login.User, login.URL, login.Name) diff --git a/modules/task/pull_create.go b/modules/task/pull_create.go index c588068..b1f2e7a 100644 --- a/modules/task/pull_create.go +++ b/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)