Skip to content

Commit

Permalink
feat: gRPC status codes and improved error messages (#467)
Browse files Browse the repository at this point in the history
  • Loading branch information
zepatrik authored Mar 8, 2021
1 parent 5f25bce commit 4a4f8c6
Show file tree
Hide file tree
Showing 11 changed files with 402 additions and 256 deletions.
2 changes: 1 addition & 1 deletion cmd/relationtuple/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func newGetCmd() *cobra.Command {
})
if err != nil {
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could not make request: %s\n", err)
return err
return cmdx.FailSilently(cmd)
}

cmdx.PrintTable(cmd, &responseOutput{
Expand Down
39 changes: 20 additions & 19 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,55 +8,56 @@ replace github.com/ory/dockertest/v3 => github.com/ory/dockertest/v3 v3.6.3

require (
github.com/HdrHistogram/hdrhistogram-go v1.0.1 // indirect
github.com/asaskevich/govalidator v0.0.0-20200428143746-21a406dcc535 // indirect
github.com/bufbuild/buf v0.31.1
github.com/cenkalti/backoff/v3 v3.0.0
github.com/containerd/continuity v0.0.0-20200228182428-0f16d7a0959c // indirect
github.com/ghodss/yaml v1.0.0
github.com/go-openapi/errors v0.19.4
github.com/go-openapi/runtime v0.19.5
github.com/go-openapi/strfmt v0.19.5
github.com/go-openapi/swag v0.19.5
github.com/go-openapi/validate v0.19.3
github.com/go-swagger/go-swagger v0.21.1-0.20200107003254-1c98855b472d
github.com/go-openapi/errors v0.20.0
github.com/go-openapi/runtime v0.19.26
github.com/go-openapi/strfmt v0.20.0
github.com/go-openapi/swag v0.19.14
github.com/go-openapi/validate v0.20.2
github.com/go-swagger/go-swagger v0.26.1
github.com/gobuffalo/pop/v5 v5.3.3
github.com/golang/protobuf v1.4.3
github.com/gorilla/sessions v1.1.3
github.com/gorilla/websocket v1.4.2
github.com/grpc-ecosystem/go-grpc-middleware v1.0.0
github.com/julienschmidt/httprouter v1.2.0
github.com/mailru/easyjson v0.7.7 // indirect
github.com/markbates/pkger v0.17.1
github.com/ory/cli v0.0.11
github.com/ory/go-acc v0.2.3
github.com/ory/go-acc v0.2.6
github.com/ory/graceful v0.1.1
github.com/ory/herodot v0.9.1
github.com/ory/herodot v0.9.2
github.com/ory/jsonschema/v3 v3.0.1
github.com/ory/x v0.0.194
github.com/pelletier/go-toml v1.8.0
github.com/ory/x v0.0.199
github.com/pelletier/go-toml v1.8.1
github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2
github.com/pkg/errors v0.9.1
github.com/rs/cors v1.6.0
github.com/segmentio/objconv v1.0.1
github.com/shopspring/decimal v1.2.0 // indirect
github.com/sirupsen/logrus v1.6.0
github.com/soheilhy/cmux v0.1.4
github.com/spf13/afero v1.5.1 // indirect
github.com/spf13/cobra v1.0.1-0.20201006035406-b97b5ead31f7
github.com/spf13/pflag v1.0.5
github.com/sqs/goreturns v0.0.0-20181028201513-538ac6014518
github.com/stretchr/testify v1.6.1
github.com/stretchr/testify v1.7.0
github.com/tidwall/gjson v1.6.0
github.com/tidwall/sjson v1.1.1
github.com/uber/jaeger-lib v2.4.0+incompatible // indirect
github.com/urfave/negroni v1.0.0
go.mongodb.org/mongo-driver v1.3.4 // indirect
golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208
golang.org/x/tools v0.0.0-20200717024301-6ddee64345a6
golang.org/x/mod v0.4.1 // indirect
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110 // indirect
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9
golang.org/x/sys v0.0.0-20210305034016-7844c3c200c3 // indirect
golang.org/x/tools v0.1.0
google.golang.org/genproto v0.0.0-20201117123952-62d171c70ae1
google.golang.org/grpc v1.35.0
google.golang.org/grpc v1.36.0
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0
google.golang.org/grpc/examples v0.0.0-20210116000752-504caa93c539 // indirect
google.golang.org/protobuf v1.25.1-0.20201020201750-d3470999428b
)

go 1.15
go 1.16
526 changes: 298 additions & 228 deletions go.sum

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion internal/driver/config/namespace_memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (s *memoryNamespaceManager) GetNamespace(_ context.Context, name string) (*
}
}

return nil, errors.WithStack(herodot.ErrNotFound)
return nil, errors.WithStack(herodot.ErrNotFound.WithReason("unknown namespace " + name))
}

func (s *memoryNamespaceManager) Namespaces(_ context.Context) ([]*namespace.Namespace, error) {
Expand Down
2 changes: 1 addition & 1 deletion internal/driver/config/namespace_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func (n *NamespaceWatcher) GetNamespace(_ context.Context, name string) (*namesp
}
}

return nil, errors.WithStack(herodot.ErrNotFound)
return nil, errors.WithStack(herodot.ErrNotFound.WithError("unknown namespace " + name))
}

func (n *NamespaceWatcher) Namespaces(_ context.Context) ([]*namespace.Namespace, error) {
Expand Down
12 changes: 8 additions & 4 deletions internal/driver/registry_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,14 @@ func (r *RegistryDefault) WriteRouter() http.Handler {

func (r *RegistryDefault) ReadGRPCServer() *grpc.Server {
s := grpc.NewServer(
grpc.StreamInterceptor(
grpc.ChainStreamInterceptor(
herodot.StreamErrorUnwrapInterceptor,
grpcMiddleware.ChainStreamServer(
grpc_logrus.StreamServerInterceptor(r.l.Entry),
),
),
grpc.UnaryInterceptor(
grpc.ChainUnaryInterceptor(
herodot.UnaryErrorUnwrapInterceptor,
grpcMiddleware.ChainUnaryServer(
grpc_logrus.UnaryServerInterceptor(r.l.Entry),
),
Expand All @@ -249,12 +251,14 @@ func (r *RegistryDefault) ReadGRPCServer() *grpc.Server {

func (r *RegistryDefault) WriteGRPCServer() *grpc.Server {
s := grpc.NewServer(
grpc.StreamInterceptor(
grpc.ChainStreamInterceptor(
herodot.StreamErrorUnwrapInterceptor,
grpcMiddleware.ChainStreamServer(
grpc_logrus.StreamServerInterceptor(r.l.Entry),
),
),
grpc.UnaryInterceptor(
grpc.ChainUnaryInterceptor(
herodot.UnaryErrorUnwrapInterceptor,
grpcMiddleware.ChainUnaryServer(
grpc_logrus.UnaryServerInterceptor(r.l.Entry),
),
Expand Down
6 changes: 6 additions & 0 deletions internal/e2e/cases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"strconv"
"testing"

"github.com/ory/herodot"

"github.com/ory/keto/internal/x"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -125,5 +127,9 @@ func runCases(c client, nspaces []*namespace.Namespace) func(*testing.T) {
resp = c.queryTuple(t, (*relationtuple.RelationQuery)(rt))
assert.Len(t, resp.RelationTuples, 0)
})

t.Run("case=returns error with status code on unknown namespace", func(t *testing.T) {
c.queryTupleErr(t, herodot.ErrNotFound, &relationtuple.RelationQuery{Namespace: "unknown namespace"})
})
}
}
15 changes: 13 additions & 2 deletions internal/e2e/cli_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"strconv"
"time"

"github.com/ory/herodot"

"github.com/ory/keto/internal/check"

grpcHealthV1 "google.golang.org/grpc/health/grpc_health_v1"
Expand Down Expand Up @@ -43,7 +45,7 @@ func (g *cliClient) createTuple(t require.TestingT, r *relationtuple.InternalRel
assert.Len(t, stderr, 0, stdout)
}

func (g *cliClient) queryTuple(t require.TestingT, q *relationtuple.RelationQuery, opts ...x.PaginationOptionSetter) *relationtuple.GetResponse {
func (g *cliClient) assembleQueryFlags(q *relationtuple.RelationQuery, opts []x.PaginationOptionSetter) []string {
var flags []string
if q.Subject != nil {
flags = append(flags, "--"+clirelationtuple.FlagSubject, q.Subject.String())
Expand All @@ -61,15 +63,24 @@ func (g *cliClient) queryTuple(t require.TestingT, q *relationtuple.RelationQuer
if pagination.Size != 0 {
flags = append(flags, "--"+clirelationtuple.FlagPageSize, strconv.Itoa(pagination.Size))
}
return flags
}

out := g.c.ExecNoErr(t, append(flags, "relation-tuple", "get", q.Namespace)...)
func (g *cliClient) queryTuple(t require.TestingT, q *relationtuple.RelationQuery, opts ...x.PaginationOptionSetter) *relationtuple.GetResponse {
out := g.c.ExecNoErr(t, append(g.assembleQueryFlags(q, opts), "relation-tuple", "get", q.Namespace)...)

var resp relationtuple.GetResponse
require.NoError(t, json.Unmarshal([]byte(out), &resp), "%s", out)

return &resp
}

func (g *cliClient) queryTupleErr(t require.TestingT, expected herodot.DefaultError, q *relationtuple.RelationQuery, opts ...x.PaginationOptionSetter) {
stdErr := g.c.ExecExpectedErr(t, append(g.assembleQueryFlags(q, opts), "relation-tuple", "get", q.Namespace)...)
assert.Contains(t, stdErr, expected.GRPCCodeField.String())
assert.Contains(t, stdErr, expected.Error())
}

func (g *cliClient) check(t require.TestingT, r *relationtuple.InternalRelationTuple) bool {
out := g.c.ExecNoErr(t, "check", r.Subject.String(), r.Relation, r.Namespace, r.Object)
var res check.RESTResponse
Expand Down
3 changes: 3 additions & 0 deletions internal/e2e/full_suit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"testing"

"github.com/ory/herodot"

"github.com/ory/keto/internal/x"

"github.com/stretchr/testify/require"
Expand All @@ -24,6 +26,7 @@ type (
deleteTuple(t require.TestingT, r *relationtuple.InternalRelationTuple)
transactTuples(t require.TestingT, ins []*relationtuple.InternalRelationTuple, del []*relationtuple.InternalRelationTuple)
queryTuple(t require.TestingT, q *relationtuple.RelationQuery, opts ...x.PaginationOptionSetter) *relationtuple.GetResponse
queryTupleErr(t require.TestingT, expected herodot.DefaultError, q *relationtuple.RelationQuery, opts ...x.PaginationOptionSetter)
check(t require.TestingT, r *relationtuple.InternalRelationTuple) bool
expand(t require.TestingT, r *relationtuple.SubjectSet, depth int) *expand.Tree
waitUntilLive(t require.TestingT)
Expand Down
29 changes: 29 additions & 0 deletions internal/e2e/grpc_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import (
"context"
"time"

"github.com/ory/herodot"
"github.com/stretchr/testify/assert"
"google.golang.org/grpc/status"

"github.com/stretchr/testify/require"
"google.golang.org/grpc"
grpcHealthV1 "google.golang.org/grpc/health/grpc_health_v1"
Expand Down Expand Up @@ -84,6 +88,31 @@ func (g *grpcClient) queryTuple(t require.TestingT, q *relationtuple.RelationQue
}
}

func (g *grpcClient) queryTupleErr(t require.TestingT, expected herodot.DefaultError, q *relationtuple.RelationQuery, opts ...x.PaginationOptionSetter) {
c := acl.NewReadServiceClient(g.readConn(t))

query := &acl.ListRelationTuplesRequest_Query{
Namespace: q.Namespace,
Object: q.Object,
Relation: q.Relation,
}
if q.Subject != nil {
query.Subject = q.Subject.ToProto()
}

pagination := x.GetPaginationOptions(opts...)

_, err := c.ListRelationTuples(g.ctx, &acl.ListRelationTuplesRequest{
Query: query,
PageToken: pagination.Token,
PageSize: int32(pagination.Size),
})
require.Error(t, err)
s, ok := status.FromError(err)
require.True(t, ok)
assert.Equal(t, expected.GRPCCodeField, s.Code(), "%+v", err)
}

func (g *grpcClient) check(t require.TestingT, r *relationtuple.InternalRelationTuple) bool {
c := acl.NewCheckServiceClient(g.readConn(t))

Expand Down
22 changes: 22 additions & 0 deletions internal/e2e/rest_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import (
"strconv"
"time"

"github.com/ory/herodot"
"github.com/tidwall/gjson"

"github.com/ory/x/healthx"

"github.com/ory/keto/internal/x"
Expand Down Expand Up @@ -84,6 +87,25 @@ func (rc *restClient) queryTuple(t require.TestingT, q *relationtuple.RelationQu
return &dec
}

func (rc *restClient) queryTupleErr(t require.TestingT, expected herodot.DefaultError, q *relationtuple.RelationQuery, opts ...x.PaginationOptionSetter) {
urlQuery := q.ToURLQuery()

pagination := x.GetPaginationOptions(opts...)
if pagination.Size != 0 {
urlQuery.Set("page_size", strconv.Itoa(pagination.Size))
}
if pagination.Token != "" {
urlQuery.Set("page_token", pagination.Token)
}

body, code := rc.makeRequest(t, http.MethodGet, fmt.Sprintf("%s?%s", relationtuple.RouteBase, urlQuery.Encode()), "", false)

assert.Equal(t, expected.CodeField, code)
assert.Equal(t, int64(expected.StatusCode()), gjson.Get(body, "error.code").Int())
assert.Equal(t, expected.Status(), gjson.Get(body, "error.status").String())
assert.Equal(t, expected.Error(), gjson.Get(body, "error.message").String(), body)
}

func (rc *restClient) check(t require.TestingT, r *relationtuple.InternalRelationTuple) bool {
bodyGet, codeGet := rc.makeRequest(t, http.MethodGet, fmt.Sprintf("%s?%s", check.RouteBase, r.ToURLQuery().Encode()), "", false)

Expand Down

0 comments on commit 4a4f8c6

Please sign in to comment.