Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to golangci-lint v1.60.1 #570

Merged
merged 1 commit into from
Aug 20, 2024
Merged

Upgrade to golangci-lint v1.60.1 #570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ check-protos: clean-protos protos ## Re-generates protos and git diffs them
GOPATH=$(CURDIR)/.tools go install github.com/fatih/faillint@v1.13.0

.tools/bin/golangci-lint: .tools
GOPATH=$(CURDIR)/.tools go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.59.1
GOPATH=$(CURDIR)/.tools go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.60.1

.tools/bin/protoc: .tools
ifeq ("$(wildcard .tools/protoc/bin/protoc)","")
Expand Down
13 changes: 7 additions & 6 deletions grpcutil/status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package grpcutil

import (
"context"
"errors"
"fmt"
"net/http"
"testing"
Expand All @@ -26,10 +27,10 @@ func TestErrorToStatus(t *testing.T) {
err: nil,
},
"a random error cannot be cast to status.Status": {
err: fmt.Errorf(msgErr),
err: errors.New(msgErr),
},
"a wrapped error of a random error cannot be cast to status.Status": {
err: fmt.Errorf("wrapped: %w", fmt.Errorf(msgErr)),
err: fmt.Errorf("wrapped: %w", errors.New(msgErr)),
},
"a gRPC error built by gogo/status can be cast to status.Status": {
err: status.Error(codes.Internal, msgErr),
Expand Down Expand Up @@ -74,11 +75,11 @@ func TestErrorToStatusCode(t *testing.T) {
expectedStatusCode: codes.OK,
},
"a non-gRPC error returns codes.Unknown": {
err: fmt.Errorf(msgErr),
err: errors.New(msgErr),
expectedStatusCode: codes.Unknown,
},
"a wrapped non-gRPC error returns codes.Unknown": {
err: fmt.Errorf("wrapped: %w", fmt.Errorf(msgErr)),
err: fmt.Errorf("wrapped: %w", errors.New(msgErr)),
expectedStatusCode: codes.Unknown,
},
"a gRPC error built by gogo/status returns its code": {
Expand Down Expand Up @@ -132,11 +133,11 @@ func TestIsCanceled(t *testing.T) {
expectedOutcome: true,
},
"a random error returns false": {
err: fmt.Errorf(msgErr),
err: errors.New(msgErr),
expectedOutcome: false,
},
"a wrapped random error returns false": {
err: fmt.Errorf("wrapped: %w", fmt.Errorf(msgErr)),
err: fmt.Errorf("wrapped: %w", errors.New(msgErr)),
expectedOutcome: false,
},
"a gRPC error with code different from codes.Canceled returns false": {
Expand Down
13 changes: 10 additions & 3 deletions httpgrpc/httpgrpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,23 @@ func FromHeader(hs http.Header) []*Header {
return result
}

// Errorf returns a HTTP gRPC error than is correctly forwarded over
// Error returns a HTTP gRPC error that is correctly forwarded over
// gRPC, and can eventually be converted back to a HTTP response with
// HTTPResponseFromError.
func Errorf(code int, tmpl string, args ...interface{}) error {
func Error(code int, msg string) error {
return ErrorFromHTTPResponse(&HTTPResponse{
Code: int32(code),
Body: []byte(fmt.Sprintf(tmpl, args...)),
Body: []byte(msg),
})
}

// Errorf returns a HTTP gRPC error that is correctly forwarded over
// gRPC, and can eventually be converted back to a HTTP response with
// HTTPResponseFromError.
func Errorf(code int, tmpl string, args ...interface{}) error {
return Error(code, fmt.Sprintf(tmpl, args...))
}

// ErrorFromHTTPResponse converts an HTTP response into a grpc error, and uses HTTP response body as an error message.
// Note that if HTTP response body contains non-utf8 string, then returned error cannot be marshalled by protobuf.
func ErrorFromHTTPResponse(resp *HTTPResponse) error {
Expand Down
9 changes: 5 additions & 4 deletions httpgrpc/httpgrpc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package httpgrpc

import (
"context"
"errors"
"fmt"
"net/http"
"testing"
Expand Down Expand Up @@ -52,7 +53,7 @@ func TestErrorf(t *testing.T) {
Code: int32(code),
Body: []byte(errMsg),
}
err := Errorf(code, errMsg)
err := Error(code, errMsg)
stat, ok := status.FromError(err)
require.True(t, ok)
require.Equal(t, code, int(stat.Code()))
Expand Down Expand Up @@ -90,7 +91,7 @@ func TestHTTPResponseFromError(t *testing.T) {
err: nil,
},
"a random error cannot be parsed to an HTTPResponse": {
err: fmt.Errorf(msgErr),
err: errors.New(msgErr),
},
"a gRPC error built by gogo/status cannot be parsed to an HTTPResponse": {
err: status.Error(codes.Internal, msgErr),
Expand All @@ -99,11 +100,11 @@ func TestHTTPResponseFromError(t *testing.T) {
err: grpcstatus.Error(codes.Internal, msgErr),
},
"a gRPC error built by httpgrpc can be parsed to an HTTPResponse": {
err: Errorf(400, msgErr),
err: Error(400, msgErr),
expectedHTTPResponse: &HTTPResponse{Code: 400, Body: []byte(msgErr)},
},
"a wrapped gRPC error built by httpgrpc can be parsed to an HTTPResponse": {
err: fmt.Errorf("wrapped: %w", Errorf(400, msgErr)),
err: fmt.Errorf("wrapped: %w", Error(400, msgErr)),
expectedHTTPResponse: &HTTPResponse{Code: 400, Body: []byte(msgErr)},
},
}
Expand Down
6 changes: 3 additions & 3 deletions middleware/grpc_instrumentation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,11 +219,11 @@ func TestInstrumentationLabel_ErrorToStatusCode(t *testing.T) {
expectedGRPCStatueCodes: codes.FailedPrecondition,
},
"a gRPC error with status codes.Canceled returns codes.Canceled": {
err: status.Errorf(codes.Canceled, context.Canceled.Error()),
err: status.Error(codes.Canceled, context.Canceled.Error()),
expectedGRPCStatueCodes: codes.Canceled,
},
"a wrapped gRPC error with status codes.Canceled returns codes.Canceled": {
err: fmt.Errorf("wrapped: %w", status.Errorf(codes.Canceled, context.Canceled.Error())),
err: fmt.Errorf("wrapped: %w", status.Error(codes.Canceled, context.Canceled.Error())),
expectedGRPCStatueCodes: codes.Canceled,
},
"context.Canceled returns codes.Canceled": {
Expand All @@ -239,7 +239,7 @@ func TestInstrumentationLabel_ErrorToStatusCode(t *testing.T) {
expectedGRPCStatueCodes: codes.Unknown,
},
"a non-gRPC error returns codes.Unknown": {
err: fmt.Errorf(errMsg),
err: errors.New(errMsg),
expectedGRPCStatueCodes: codes.Unknown,
},
}
Expand Down
2 changes: 1 addition & 1 deletion ring/replication_set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ func DoUntilQuorumWithoutSuccessfulContextCancellation[T any](ctx context.Contex
ext.Error.Set(cfg.Logger.Span, true)
}

contextTracker.cancelAllContexts(cancellation.NewErrorf(cause))
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't see why the new version is better.

Copy link
Collaborator Author

@aknuds1 aknuds1 Aug 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's generally because the linter thinks the non-formatting version should be used, if there's no string formatting. Why do you eventually think the linter is wrong about this?

contextTracker.cancelAllContexts(cancellation.NewError(errors.New(cause)))
cleanupResultsAlreadyReceived()
return nil, err
}
Expand Down
3 changes: 1 addition & 2 deletions server/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"os"
"os/exec"
"path/filepath"
"strconv"
"testing"
"time"

Expand Down Expand Up @@ -48,7 +47,7 @@ func (f FakeServer) FailWithError(_ context.Context, _ *protobuf.Empty) (*protob
}

func (f FakeServer) FailWithHTTPError(_ context.Context, req *FailWithHTTPErrorRequest) (*protobuf.Empty, error) {
return nil, httpgrpc.Errorf(int(req.Code), strconv.Itoa(int(req.Code)))
return nil, httpgrpc.Errorf(int(req.Code), "%d", req.Code)
}

func (f FakeServer) Succeed(_ context.Context, _ *protobuf.Empty) (*protobuf.Empty, error) {
Expand Down