Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 24 additions & 0 deletions cmd/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/samzong/gmc/internal/config"
"github.com/samzong/gmc/internal/exitcode"
"github.com/samzong/gmc/internal/git"
"github.com/samzong/gmc/internal/llm"
"github.com/samzong/gmc/internal/workflow"
Expand Down Expand Up @@ -60,6 +61,29 @@ func TestHandleErrors(t *testing.T) {
assert.ErrorIs(t, errWithoutHint, workflow.ErrNoChanges)
})

t.Run("not git repo returns exit code 11", func(t *testing.T) {
err := handleErrors(fmt.Errorf("failed: %w", git.ErrNotGitRepo), false)
var exitErr *exitcode.Error
assert.ErrorAs(t, err, &exitErr)
assert.Equal(t, exitcode.NotGitRepo, exitErr.Code)
assert.ErrorIs(t, err, git.ErrNotGitRepo)
})

t.Run("LLM error returns exit code 12", func(t *testing.T) {
err := handleErrors(fmt.Errorf("generation failed: %w", llm.ErrLLM), false)
var exitErr *exitcode.Error
assert.ErrorAs(t, err, &exitErr)
assert.Equal(t, exitcode.LLMError, exitErr.Code)
assert.ErrorIs(t, err, llm.ErrLLM)
})

t.Run("no staged changes returns exit code 10", func(t *testing.T) {
err := handleErrors(workflow.ErrNoChanges, false)
var exitErr *exitcode.Error
assert.ErrorAs(t, err, &exitErr)
assert.Equal(t, exitcode.NoStagedChanges, exitErr.Code)
})

t.Run("propagates generic error", func(t *testing.T) {
expectedErr := errors.New("boom")
err := handleErrors(expectedErr, false)
Expand Down
17 changes: 16 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/mattn/go-isatty"
"github.com/samzong/gmc/internal/config"
"github.com/samzong/gmc/internal/exitcode"
"github.com/samzong/gmc/internal/formatter"
"github.com/samzong/gmc/internal/git"
"github.com/samzong/gmc/internal/llm"
Expand Down Expand Up @@ -99,12 +100,26 @@ func handleErrors(err error, addAllFlag bool) error {
if !addAllFlag {
msg += "\nHint: You can use -a or --all to automatically add all changes to the staging area."
}
return userFacingError{msg: msg, err: err}
return exitcode.New(exitcode.NoStagedChanges, msg, err)
}

if coded := classifyError(err); coded != nil {
return coded
}

return userFacingError{msg: fmt.Sprintf("gmc: %v", err), err: err}
}

func classifyError(err error) *exitcode.Error {
if errors.Is(err, git.ErrNotGitRepo) {
return exitcode.New(exitcode.NotGitRepo, err.Error(), err)
}
if errors.Is(err, llm.ErrLLM) {
return exitcode.New(exitcode.LLMError, err.Error(), err)
}
return nil
}

type userFacingError struct {
msg string
err error
Expand Down
17 changes: 12 additions & 5 deletions cmd/tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func runTagCommand() error {

lastTag, commits, err := collectTagContext(gitClient)
if err != nil {
return err
return wrapTagError(err)
}
if len(commits) == 0 {
printNoCommitsSinceLastTag(lastTag)
Expand All @@ -67,14 +67,14 @@ func runTagCommand() error {

baseVersion, displayTag, err := resolveBaseVersion(lastTag)
if err != nil {
return err
return wrapTagError(err)
}

printCommitSummary(displayTag, commits)

finalVersion, finalReason, source, err := pickTagSuggestion(baseVersion, commits, llmClient)
if err != nil {
return err
return wrapTagError(err)
}
fmt.Fprintf(outWriter(), "Suggested version (%s): %s\n", source, finalVersion.String())
if strings.TrimSpace(finalReason) != "" {
Expand All @@ -89,7 +89,7 @@ func runTagCommand() error {

confirmed, err := confirmTagCreation(finalVersion.String())
if err != nil {
return fmt.Errorf("failed to read confirmation: %w", err)
return wrapTagError(fmt.Errorf("failed to read confirmation: %w", err))
}

if !confirmed {
Expand All @@ -103,7 +103,7 @@ func runTagCommand() error {
}

if err := gitClient.CreateAnnotatedTag(finalVersion.String(), tagMessage); err != nil {
return fmt.Errorf("failed to create tag: %w", err)
return wrapTagError(fmt.Errorf("failed to create tag: %w", err))
}

fmt.Fprintf(outWriter(), "Tag %s created successfully.\n", finalVersion.String())
Expand Down Expand Up @@ -276,3 +276,10 @@ func confirmTagCreation(tag string) (bool, error) {
answer := strings.TrimSpace(strings.ToLower(input))
return answer == "y" || answer == "yes", nil
}

func wrapTagError(err error) error {
if coded := classifyError(err); coded != nil {
return coded
}
return err
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To ensure consistent error message formatting across all commands, unclassified errors from the tag command should also be wrapped in userFacingError, similar to how the root command handles them. This will provide a uniform experience for users, with errors prefixed by gmc: .

After making this change, please also ensure all error return paths in runTagCommand use this wrapper for consistency (e.g., the one at line 70 seems to have been missed).

Suggested change
return err
return userFacingError{msg: fmt.Sprintf("gmc: %v", err), err: err}

}
23 changes: 23 additions & 0 deletions internal/exitcode/exitcode.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package exitcode

const (
Success = 0
General = 1
Usage = 2
NoStagedChanges = 10
NotGitRepo = 11
LLMError = 12
)

type Error struct {
Code int
Message string
Err error
}

func (e *Error) Error() string { return e.Message }
func (e *Error) Unwrap() error { return e.Err }

func New(code int, msg string, err error) *Error {
return &Error{Code: code, Message: msg, Err: err}
}
4 changes: 3 additions & 1 deletion internal/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,11 @@ func (c *Client) IsGitRepository() bool {
return err == nil
}

var ErrNotGitRepo = errors.New("not a git repository")

func (c *Client) CheckGitRepository() error {
if !c.IsGitRepository() {
return errors.New("not in a git repository. Please run this command in a git repository directory")
return fmt.Errorf("%w: please run this command inside a git working tree", ErrNotGitRepo)
}
return nil
}
Expand Down
9 changes: 3 additions & 6 deletions internal/git/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,20 +428,17 @@ func TestOutsideGitRepo(t *testing.T) {

t.Run("CheckGitRepository_OutsideRepo", func(t *testing.T) {
err := client.CheckGitRepository()
assert.Error(t, err)
assert.Contains(t, err.Error(), "not in a git repository")
assert.ErrorIs(t, err, ErrNotGitRepo)
})

t.Run("GetDiff_OutsideRepo", func(t *testing.T) {
_, err := client.GetDiff()
assert.Error(t, err)
assert.Contains(t, err.Error(), "not in a git repository")
assert.ErrorIs(t, err, ErrNotGitRepo)
})

t.Run("GetStagedDiff_OutsideRepo", func(t *testing.T) {
_, err := client.GetStagedDiff()
assert.Error(t, err)
assert.Contains(t, err.Error(), "not in a git repository")
assert.ErrorIs(t, err, ErrNotGitRepo)
})

t.Run("AddAll_OutsideRepo", func(t *testing.T) {
Expand Down
10 changes: 6 additions & 4 deletions internal/llm/llm.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/sashabaranov/go-openai"
)

var ErrLLM = errors.New("LLM error")

type Options struct {
Timeout time.Duration
}
Expand Down Expand Up @@ -99,11 +101,11 @@ func (c *Client) GenerateCommitMessage(prompt string, model string) (string, err
)

if err != nil {
return "", fmt.Errorf("failed to call LLM: %w", err)
return "", fmt.Errorf("failed to call LLM: %w (%w)", err, ErrLLM)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The use of multiple %w format verbs for error wrapping was introduced in Go 1.20. If this project needs to maintain compatibility with older Go versions, this will cause a compilation error.

For compatibility with Go versions < 1.20, you could wrap only ErrLLM and include the original error's text in the message. This preserves the ability to check for ErrLLM while keeping the context.

For example:

return "", fmt.Errorf("failed to call LLM: %v: %w", err, ErrLLM)

This would mean errors.As would no longer work for the original err, but errors.Is for ErrLLM would still function correctly.

}

if len(resp.Choices) == 0 {
return "", errors.New("LLM returned empty response")
return "", fmt.Errorf("LLM returned empty response: %w", ErrLLM)
}

return strings.TrimSpace(resp.Choices[0].Message.Content), nil
Expand Down Expand Up @@ -143,11 +145,11 @@ func (c *Client) SuggestVersion(baseVersion string, commits []string, model stri
)

if err != nil {
return "", "", fmt.Errorf("failed to call LLM: %w", err)
return "", "", fmt.Errorf("failed to call LLM: %w (%w)", err, ErrLLM)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Similar to the comment on line 104, the use of multiple %w verbs for error wrapping requires Go 1.20+. If compatibility with older versions is needed, consider an alternative wrapping strategy.

For example:

return "", "", fmt.Errorf("failed to call LLM: %v: %w", err, ErrLLM)

}

if len(resp.Choices) == 0 {
return "", "", errors.New("LLM returned empty response")
return "", "", fmt.Errorf("LLM returned empty response: %w", ErrLLM)
}

version, reason, err := parseVersionSuggestion(resp.Choices[0].Message.Content)
Expand Down
8 changes: 7 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
package main

import (
"errors"
"os"

"github.com/samzong/gmc/cmd"
"github.com/samzong/gmc/internal/exitcode"
)

func main() {
if err := cmd.Execute(); err != nil {
os.Exit(1)
var exitErr *exitcode.Error
if errors.As(err, &exitErr) {
os.Exit(exitErr.Code)
}
os.Exit(exitcode.General)
}
}
Loading