fix(milestone): return the base repository error instead of panicking

What does this MR do and why?

glab milestone get, create, edit and delete crash instead of reporting an error when the base repository cannot be resolved. All four discard the error from baseRepo() and dereference the nil repository on the next line:

repo, _ := o.baseRepo()
milestone, _, err := client.Milestones.GetMilestone(repo.FullName(), o.milestoneID)

Running any of them outside a Git repository, without --project or --group, produces:

$ cd /tmp/not-a-repo && glab milestone get 1
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x18 pc=0x7ff77c64bc3a]

goroutine 1 [running]:
gitlab.com/gitlab-org/cli/internal/commands/milestone/get.(*options).run(...)
	internal/commands/milestone/get/get.go:119 +0x29a

baseRepo() already returns a usable error here. This MR returns it, matching how the rest of the codebase handles a failed repository lookup.

Fixed at:

  • internal/commands/milestone/create/create.go:146
  • internal/commands/milestone/get/get.go:118
  • internal/commands/milestone/edit/edit.go:116
  • internal/commands/milestone/delete/delete.go:100

Test coverage

One regression test per command, using the existing cmdtest.WithBaseRepoError factory option:

exec := cmdtest.SetupCmdForTest(t, NewCmdGet, false,
	cmdtest.WithApiClient(...),
	cmdtest.WithBaseRepoError(errors.New("no base repository")),
)
_, err := exec("123")
require.Error(t, err)

Reverting only the four non-test files makes all four tests panic:

--- FAIL: Test_CreateProjectMilestone_BaseRepoError (0.02s)
panic: runtime error: invalid memory address or nil pointer dereference
--- FAIL: Test_DeleteProjectMilestone_BaseRepoError (0.01s)
panic: runtime error: invalid memory address or nil pointer dereference
--- FAIL: Test_EditProjectMilestone_BaseRepoError (0.01s)
panic: runtime error: invalid memory address or nil pointer dereference

With the fix, go test ./internal/commands/milestone/... is green.

The original panic was also reproduced against a real build of the binary, not only in tests.

Closes #8415 (closed)

Merge request reports

Loading
Loading