Skip to content

Commit f9d6cd6

Browse files
tomowatttomowatt
authored
SUP-7062: use errors.As with gqlerror.List for GraphQL error handling (#1144)
1 parent 7b6a8e9 commit f9d6cd6

6 files changed

Lines changed: 131 additions & 47 deletions

File tree

‎buildkite/resource_organization_banner.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"fmt"
66
"log"
7-
"strings"
87

98
"github.com/MakeNowJust/heredoc"
109
"github.com/hashicorp/terraform-plugin-framework/path"
@@ -247,7 +246,7 @@ func (ob *organizationBannerResource) Delete(ctx context.Context, req resource.D
247246
})
248247
if err != nil {
249248
// Handle the case where banner was already deleted (doesn't exist)
250-
if strings.Contains(err.Error(), "does not have an active banner") {
249+
if gqlErrorContains(err, "does not have an active banner") {
251250
log.Printf("Organization banner %s already deleted", state.ID.ValueString())
252251
resp.State.RemoveResource(ctx)
253252
return

‎buildkite/resource_pipeline_webhook.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"errors"
66
"fmt"
7-
"strings"
87

98
"github.com/MakeNowJust/heredoc"
109
"github.com/hashicorp/terraform-plugin-framework/path"
@@ -137,7 +136,7 @@ func (pw *pipelineWebhook) Create(ctx context.Context, req resource.CreateReques
137136
err = retry.RetryContext(ctx, timeouts, func() *retry.RetryError {
138137
apiResponse, err := createPipelineWebhook(ctx, pw.client.genqlient, plan.PipelineId.ValueString())
139138
if err != nil {
140-
if strings.Contains(err.Error(), "A webhook already exists for this repository") {
139+
if gqlErrorContains(err, "A webhook already exists for this repository") {
141140
readResp, readErr := getPipelineWebhook(ctx, pw.client.genqlient, plan.PipelineId.ValueString())
142141
if readErr != nil {
143142
return retry.NonRetryableError(fmt.Errorf("webhook exists but failed to read: %w", readErr))

‎buildkite/util.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package buildkite
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"log"
78
"os"
@@ -11,6 +12,7 @@ import (
1112
"github.com/hashicorp/terraform-plugin-framework/types"
1213
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
1314
"github.com/shurcooL/graphql"
15+
"github.com/vektah/gqlparser/v2/gqlerror"
1416
)
1517

1618
var (
@@ -24,6 +26,15 @@ func isResourceNotFoundError(err error) bool {
2426
if err == nil {
2527
return false
2628
}
29+
var errList gqlerror.List
30+
if errors.As(err, &errList) {
31+
for _, e := range errList {
32+
if resourceNotFoundRegex.MatchString(e.Message) {
33+
return true
34+
}
35+
}
36+
return false
37+
}
2738
return resourceNotFoundRegex.MatchString(err.Error())
2839
}
2940

@@ -32,6 +43,15 @@ func isActiveJobsError(err error) bool {
3243
if err == nil {
3344
return false
3445
}
46+
var errList gqlerror.List
47+
if errors.As(err, &errList) {
48+
for _, e := range errList {
49+
if activeJobsRegex.MatchString(e.Message) {
50+
return true
51+
}
52+
}
53+
return false
54+
}
3555
return activeJobsRegex.MatchString(err.Error())
3656
}
3757

@@ -40,9 +60,36 @@ func isAlreadyExistsError(err error) bool {
4060
if err == nil {
4161
return false
4262
}
63+
var errList gqlerror.List
64+
if errors.As(err, &errList) {
65+
for _, e := range errList {
66+
if alreadyExistsRegex.MatchString(e.Message) {
67+
return true
68+
}
69+
}
70+
return false
71+
}
4372
return alreadyExistsRegex.MatchString(err.Error())
4473
}
4574

75+
// gqlErrorContains reports whether any GraphQL error in err contains substring s.
76+
// Falls back to strings.Contains(err.Error(), s) for non-gqlerror errors.
77+
func gqlErrorContains(err error, s string) bool {
78+
if err == nil {
79+
return false
80+
}
81+
var errList gqlerror.List
82+
if errors.As(err, &errList) {
83+
for _, e := range errList {
84+
if strings.Contains(e.Message, s) {
85+
return true
86+
}
87+
}
88+
return false
89+
}
90+
return strings.Contains(err.Error(), s)
91+
}
92+
4693
// GetOrganizationID retrieves the Buildkite organization ID associated with the supplied slug
4794
func GetOrganizationID(slug string, client *graphql.Client) (string, error) {
4895
var query struct {

‎buildkite/util_test.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"errors"
55
"os"
66
"testing"
7+
8+
"github.com/vektah/gqlparser/v2/gqlerror"
79
)
810

911
func TestGetOrganizationIDMissing(t *testing.T) {
@@ -27,6 +29,59 @@ func TestGetOrganizationIDMissing(t *testing.T) {
2729
}
2830
}
2931

32+
func TestIsResourceNotFoundError(t *testing.T) {
33+
tests := []struct {
34+
name string
35+
err error
36+
shouldMatch bool
37+
}{
38+
{
39+
name: "nil error",
40+
err: nil,
41+
shouldMatch: false,
42+
},
43+
{
44+
name: "not found plain error",
45+
err: errors.New("Resource not found"),
46+
shouldMatch: true,
47+
},
48+
{
49+
name: "no longer exists plain error",
50+
err: errors.New("This resource no longer exists"),
51+
shouldMatch: true,
52+
},
53+
{
54+
name: "gqlerror.List not found",
55+
err: gqlerror.List{&gqlerror.Error{Message: "No Pipeline found"}},
56+
shouldMatch: true,
57+
},
58+
{
59+
name: "gqlerror.List no longer exists",
60+
err: gqlerror.List{&gqlerror.Error{Message: "This resource no longer exists"}},
61+
shouldMatch: true,
62+
},
63+
{
64+
name: "gqlerror.List unrelated",
65+
err: gqlerror.List{&gqlerror.Error{Message: "Network connection failed"}},
66+
shouldMatch: false,
67+
},
68+
{
69+
name: "unrelated plain error",
70+
err: errors.New("Network connection failed"),
71+
shouldMatch: false,
72+
},
73+
}
74+
75+
for _, tt := range tests {
76+
t.Run(tt.name, func(t *testing.T) {
77+
result := isResourceNotFoundError(tt.err)
78+
if result != tt.shouldMatch {
79+
t.Errorf("isResourceNotFoundError(%v) = %v, want %v", tt.err, result, tt.shouldMatch)
80+
}
81+
})
82+
}
83+
}
84+
3085
func TestIsAlreadyExistsError(t *testing.T) {
3186
tests := []struct {
3287
name string
@@ -63,6 +118,16 @@ func TestIsAlreadyExistsError(t *testing.T) {
63118
err: errors.New("Resource not found"),
64119
shouldMatch: false,
65120
},
121+
{
122+
name: "gqlerror.List already been added",
123+
err: gqlerror.List{&gqlerror.Error{Message: "This pipeline has already been added to this team"}},
124+
shouldMatch: true,
125+
},
126+
{
127+
name: "gqlerror.List unrelated",
128+
err: gqlerror.List{&gqlerror.Error{Message: "Network connection failed"}},
129+
shouldMatch: false,
130+
},
66131
}
67132

68133
for _, tt := range tests {
@@ -136,6 +201,16 @@ func TestIsActiveJobsError(t *testing.T) {
136201
err: errors.New("Insufficient permissions"),
137202
shouldMatch: false,
138203
},
204+
{
205+
name: "gqlerror.List active builds",
206+
err: gqlerror.List{&gqlerror.Error{Message: "Cannot delete pipeline with active builds"}},
207+
shouldMatch: true,
208+
},
209+
{
210+
name: "gqlerror.List unrelated",
211+
err: gqlerror.List{&gqlerror.Error{Message: "Insufficient permissions"}},
212+
shouldMatch: false,
213+
},
139214
}
140215

141216
for _, tt := range tests {

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ require (
7676
github.com/mitchellh/mapstructure v1.5.0 // indirect
7777
github.com/mitchellh/reflectwalk v1.0.2 // indirect
7878
github.com/oklog/run v1.2.0 // indirect
79-
github.com/vektah/gqlparser/v2 v2.5.27 // indirect
79+
github.com/vektah/gqlparser/v2 v2.5.27
8080
github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect
8181
github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect
8282
github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect

0 commit comments

Comments
 (0)