Skip to content

Commit 7be0384

Browse files
committed
refactor: use idiomatic Go error handling with errors.Is()
- Move error definitions to internal/api/errors.go - Add classifyError() to detect error patterns from GraphQL responses - Update conversation_service to wrap errors with sentinel types - Update conversation_handler to use errors.Is() instead of string matching - Remove IsNotFound/IsAlreadyExists helper functions (no longer needed) The API layer now owns error classification, consumers use errors.Is().
1 parent 9513d72 commit 7be0384

7 files changed

Lines changed: 191 additions & 159 deletions

File tree

‎internal/api/api.go‎

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,9 @@
11
package api
22

33
import (
4-
"errors"
5-
"strings"
6-
74
"github.com/usetero/cli/internal/log"
85
)
96

10-
// Sentinel errors for API responses.
11-
var (
12-
// ErrNotFound indicates the requested resource does not exist.
13-
ErrNotFound = errors.New("not found")
14-
15-
// ErrAlreadyExists indicates the resource already exists.
16-
ErrAlreadyExists = errors.New("already exists")
17-
)
18-
19-
// IsNotFound returns true if the error indicates a resource was not found.
20-
func IsNotFound(err error) bool {
21-
if err == nil {
22-
return false
23-
}
24-
if errors.Is(err, ErrNotFound) {
25-
return true
26-
}
27-
// Check error message for common "not found" patterns from GraphQL
28-
msg := strings.ToLower(err.Error())
29-
return strings.Contains(msg, "not found") || strings.Contains(msg, "not_found")
30-
}
31-
32-
// IsAlreadyExists returns true if the error indicates a resource already exists.
33-
func IsAlreadyExists(err error) bool {
34-
if err == nil {
35-
return false
36-
}
37-
if errors.Is(err, ErrAlreadyExists) {
38-
return true
39-
}
40-
// Check error message for common "already exists" patterns from GraphQL
41-
msg := strings.ToLower(err.Error())
42-
return strings.Contains(msg, "already exists") || strings.Contains(msg, "already_exists") ||
43-
strings.Contains(msg, "duplicate") || strings.Contains(msg, "conflict")
44-
}
45-
467
// API bundles all control plane API services.
478
// It provides a single entry point for all API-related operations.
489
type API struct {

‎internal/api/api_test.go‎

Lines changed: 0 additions & 116 deletions
This file was deleted.

‎internal/api/conversation_service.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package api
22

33
import (
44
"context"
5+
"errors"
6+
"fmt"
57

68
"github.com/google/uuid"
79
"github.com/usetero/cli/internal/log"
@@ -52,6 +54,9 @@ func (s *ConversationService) Create(ctx context.Context, id uuid.UUID, workspac
5254
resp, err := s.client.CreateConversation(ctx, input)
5355
if err != nil {
5456
s.logger.Error("failed to create conversation", "error", err)
57+
if classified := classifyError(err); classified != nil {
58+
return nil, fmt.Errorf("create conversation %s: %w", id, classified)
59+
}
5560
return nil, err
5661
}
5762

@@ -76,6 +81,9 @@ func (s *ConversationService) Update(ctx context.Context, id, title string) (*Co
7681
resp, err := s.client.UpdateConversation(ctx, id, input)
7782
if err != nil {
7883
s.logger.Error("failed to update conversation", "error", err, "id", id)
84+
if classified := classifyError(err); classified != nil {
85+
return nil, fmt.Errorf("update conversation %s: %w", id, classified)
86+
}
7987
return nil, err
8088
}
8189

@@ -89,12 +97,16 @@ func (s *ConversationService) Update(ctx context.Context, id, title string) (*Co
8997
}
9098

9199
// Delete deletes a conversation.
100+
// Returns ErrNotFound (via errors.Is) if the conversation does not exist.
92101
func (s *ConversationService) Delete(ctx context.Context, id string) error {
93102
s.logger.Debug("deleting conversation via API", "id", id)
94103

95104
_, err := s.client.DeleteConversation(ctx, id)
96105
if err != nil {
97106
s.logger.Error("failed to delete conversation", "error", err, "id", id)
107+
if classified := classifyError(err); classified != nil {
108+
return errors.Join(fmt.Errorf("delete conversation %s", id), classified)
109+
}
98110
return err
99111
}
100112

‎internal/api/errors.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package api
2+
3+
import (
4+
"errors"
5+
"strings"
6+
)
7+
8+
// Sentinel errors for API responses.
9+
// Use errors.Is() to check for these error types.
10+
var (
11+
// ErrNotFound indicates the requested resource does not exist.
12+
ErrNotFound = errors.New("not found")
13+
14+
// ErrAlreadyExists indicates the resource already exists.
15+
ErrAlreadyExists = errors.New("already exists")
16+
)
17+
18+
// notFoundPatterns are substrings that indicate a "not found" error from the API.
19+
var notFoundPatterns = []string{
20+
"not found",
21+
"not_found",
22+
"does not exist",
23+
}
24+
25+
// alreadyExistsPatterns are substrings that indicate an "already exists" error from the API.
26+
var alreadyExistsPatterns = []string{
27+
"already exists",
28+
"already_exists",
29+
"duplicate",
30+
"conflict",
31+
}
32+
33+
// classifyError examines an error message and returns a wrapped sentinel error
34+
// if the message matches known patterns. Returns nil if no pattern matches.
35+
func classifyError(err error) error {
36+
if err == nil {
37+
return nil
38+
}
39+
40+
msg := strings.ToLower(err.Error())
41+
42+
for _, pattern := range notFoundPatterns {
43+
if strings.Contains(msg, pattern) {
44+
return ErrNotFound
45+
}
46+
}
47+
48+
for _, pattern := range alreadyExistsPatterns {
49+
if strings.Contains(msg, pattern) {
50+
return ErrAlreadyExists
51+
}
52+
}
53+
54+
return nil
55+
}

‎internal/api/errors_test.go‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
package api
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"testing"
7+
)
8+
9+
func TestClassifyError(t *testing.T) {
10+
t.Parallel()
11+
12+
tests := []struct {
13+
name string
14+
err error
15+
want error
16+
}{
17+
{
18+
name: "nil error",
19+
err: nil,
20+
want: nil,
21+
},
22+
{
23+
name: "not found",
24+
err: errors.New("conversation not found"),
25+
want: ErrNotFound,
26+
},
27+
{
28+
name: "NOT_FOUND",
29+
err: errors.New("ERROR: NOT_FOUND"),
30+
want: ErrNotFound,
31+
},
32+
{
33+
name: "not_found",
34+
err: errors.New("resource_not_found"),
35+
want: ErrNotFound,
36+
},
37+
{
38+
name: "does not exist",
39+
err: errors.New("conversation does not exist"),
40+
want: ErrNotFound,
41+
},
42+
{
43+
name: "already exists",
44+
err: errors.New("conversation already exists"),
45+
want: ErrAlreadyExists,
46+
},
47+
{
48+
name: "ALREADY_EXISTS",
49+
err: errors.New("ERROR: ALREADY_EXISTS"),
50+
want: ErrAlreadyExists,
51+
},
52+
{
53+
name: "duplicate",
54+
err: errors.New("duplicate key violation"),
55+
want: ErrAlreadyExists,
56+
},
57+
{
58+
name: "conflict",
59+
err: errors.New("conflict: resource exists"),
60+
want: ErrAlreadyExists,
61+
},
62+
{
63+
name: "unrelated error",
64+
err: errors.New("network timeout"),
65+
want: nil,
66+
},
67+
}
68+
69+
for _, tt := range tests {
70+
t.Run(tt.name, func(t *testing.T) {
71+
t.Parallel()
72+
got := classifyError(tt.err)
73+
// classifyError returns sentinel errors directly (ErrNotFound, ErrAlreadyExists, or nil)
74+
// so direct comparison is correct here
75+
if (got == nil) != (tt.want == nil) || (got != nil && tt.want != nil && !errors.Is(got, tt.want)) {
76+
t.Errorf("classifyError() = %v, want %v", got, tt.want)
77+
}
78+
})
79+
}
80+
}
81+
82+
func TestClassifiedErrorsWorkWithErrorsIs(t *testing.T) {
83+
t.Parallel()
84+
85+
t.Run("wrapped ErrNotFound", func(t *testing.T) {
86+
t.Parallel()
87+
original := errors.New("conversation not found")
88+
classified := classifyError(original)
89+
wrapped := fmt.Errorf("delete conversation abc: %w", classified)
90+
91+
if !errors.Is(wrapped, ErrNotFound) {
92+
t.Error("errors.Is(wrapped, ErrNotFound) = false, want true")
93+
}
94+
})
95+
96+
t.Run("wrapped ErrAlreadyExists", func(t *testing.T) {
97+
t.Parallel()
98+
original := errors.New("conversation already exists")
99+
classified := classifyError(original)
100+
wrapped := fmt.Errorf("create conversation abc: %w", classified)
101+
102+
if !errors.Is(wrapped, ErrAlreadyExists) {
103+
t.Error("errors.Is(wrapped, ErrAlreadyExists) = false, want true")
104+
}
105+
})
106+
107+
t.Run("errors.Join preserves both errors", func(t *testing.T) {
108+
t.Parallel()
109+
contextErr := fmt.Errorf("delete conversation %s", "abc")
110+
joined := errors.Join(contextErr, ErrNotFound)
111+
112+
if !errors.Is(joined, ErrNotFound) {
113+
t.Error("errors.Is(joined, ErrNotFound) = false, want true")
114+
}
115+
})
116+
}

0 commit comments

Comments
 (0)