From 3308bcd71a38cecd2a1fa5907a5780da49a6031b Mon Sep 17 00:00:00 2001 From: Adam Sven Johnson Date: Fri, 25 Oct 2024 10:18:45 +1300 Subject: [PATCH 1/5] Update golangci-lint --- .github/workflows/go.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 3f27d9e5..a51b8151 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -38,8 +38,8 @@ jobs: with: go-version-file: go.mod - name: Lint - uses: golangci/golangci-lint-action@v3 + uses: golangci/golangci-lint-action@v6 with: - version: v1.55.2 + version: v1.61.0 args: --disable errcheck . plugins only-new-issues: true From 3011a87d8153f641140302f96e25aacb0c3cadc3 Mon Sep 17 00:00:00 2001 From: Adam Sven Johnson Date: Fri, 25 Oct 2024 09:59:22 +1300 Subject: [PATCH 2/5] Remove unnecessary nil check len() of a nil slice returns 0 so the double check is unecessary. --- execution_result.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/execution_result.go b/execution_result.go index ff945890..fc67d763 100644 --- a/execution_result.go +++ b/execution_result.go @@ -340,7 +340,7 @@ func formatResponseDataRec(schema *ast.Schema, selectionSet ast.SelectionSet, re fieldData, ok := result[field.Alias] if !ok { innerBuf.WriteString("null") - } else if field.SelectionSet != nil && len(field.SelectionSet) > 0 { + } else if len(field.SelectionSet) > 0 { val := formatResponseDataRec(schema, field.SelectionSet, fieldData, false) innerBuf.Write(val) } else { From 9ef70de7e7863703c8281666f4254554ca858f9b Mon Sep 17 00:00:00 2001 From: Adam Sven Johnson Date: Fri, 25 Oct 2024 10:08:32 +1300 Subject: [PATCH 3/5] Remove loop variable aliasing prevention This issue is fixed in go1.22 https://tip.golang.org/doc/go1.22#language --- executable_schema.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/executable_schema.go b/executable_schema.go index 2dd7d576..205655fa 100644 --- a/executable_schema.go +++ b/executable_schema.go @@ -103,9 +103,7 @@ func (s *ExecutableSchema) UpdateSchema(ctx context.Context, forceRebuild bool) // Avoid fetching more than 64 servides in parallel, // as high concurrency can actually hurt performance group.SetLimit(64) - for url_, s_ := range s.Services { - url := url_ - s := s_ + for url, s := range s.Services { group.Go(func() error { logger := log.WithField("url", url) updated, err := s.Update(ctx) From bb32ba43f4841439df7a87b402ec66dcbaa5a195 Mon Sep 17 00:00:00 2001 From: Adam Sven Johnson Date: Fri, 25 Oct 2024 10:33:44 +1300 Subject: [PATCH 4/5] Use constant format string Cleans up a lint warning, probably wouldn't be an issue unless a malicious `error` returned a weird string. --- executable_schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executable_schema.go b/executable_schema.go index 205655fa..61f7d36c 100644 --- a/executable_schema.go +++ b/executable_schema.go @@ -221,7 +221,7 @@ func (s *ExecutableSchema) ExecuteQuery(ctx context.Context) *graphql.Response { }) if err != nil { traceErr(err) - return s.interceptResponse(ctx, operation.Name, operationCtx.RawQuery, variables, graphql.ErrorResponse(ctx, err.Error())) + return s.interceptResponse(ctx, operation.Name, operationCtx.RawQuery, variables, graphql.ErrorResponse(ctx, "%s", err.Error())) } extensions := make(map[string]interface{}) From d0dd5ec27c95e1fbd10f08b2b82805ed739bdfa5 Mon Sep 17 00:00:00 2001 From: Adam Sven Johnson Date: Tue, 29 Oct 2024 14:03:57 +1300 Subject: [PATCH 5/5] Stop wrapping http error The error message provides no extra value, the original error provides plenty of detail for example: Post "http://service.name:80/query": dial tcp 172.1.2.3:80: connect: connection refused --- client.go | 7 +------ docs/plugins.md | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/client.go b/client.go index 73e9b506..eec24054 100644 --- a/client.go +++ b/client.go @@ -162,13 +162,8 @@ func (c *GraphQLClient) Request(ctx context.Context, url string, request *Reques promServiceTimeoutErrorCounter.With(prometheus.Labels{ "service": url, }).Inc() - - // Return raw timeout error to allow caller to handle it since a - // downstream caller may want to retry, and they will have to jump - // through hoops to detect this error otherwise. - return traceErr(err) } - return traceErr(fmt.Errorf("error during request: %w", err)) + return traceErr(err) } defer res.Body.Close() diff --git a/docs/plugins.md b/docs/plugins.md index 507b8464..13f25afa 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -121,7 +121,7 @@ e.g. ```json { "errors": { - "message": "error during request: Post \"http://localhost:8080/query\": context deadline exceeded", + "message": "Post \"http://localhost:8080/query\": context deadline exceeded", "extensions": { "selectionSet": "{ serviceB _bramble_id: id }" },