From bd4718583f1feea04381bf306acc12febe75dfcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Mon, 18 Mar 2024 11:41:39 +0000 Subject: [PATCH] do return backward incompatible `--explain` debug info in trailer Previous change caused `zed` to fail to parse `--explain` trailer. This means new release causes clients to miss debug info trailer, but seems better than breaking them with an incompatible message. --- internal/services/v1/debug_test.go | 3 ++- internal/services/v1/permissions.go | 9 --------- internal/services/v1/permissions_test.go | 5 +++-- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/internal/services/v1/debug_test.go b/internal/services/v1/debug_test.go index 096821b204..dea12bb669 100644 --- a/internal/services/v1/debug_test.go +++ b/internal/services/v1/debug_test.go @@ -482,7 +482,8 @@ func TestCheckPermissionWithDebug(t *testing.T) { encodedDebugInfo, err := responsemeta.GetResponseTrailerMetadataOrNil(trailer, responsemeta.DebugInformation) req.NoError(err) - req.NotNil(encodedDebugInfo) + // DebugInfo No longer comes as part of the trailer + req.Nil(encodedDebugInfo) debugInfo := checkResp.DebugTrace req.NotEmpty(debugInfo.SchemaUsed) diff --git a/internal/services/v1/permissions.go b/internal/services/v1/permissions.go index 9d29b701fe..8e8949f6ee 100644 --- a/internal/services/v1/permissions.go +++ b/internal/services/v1/permissions.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/authzed/authzed-go/pkg/requestmeta" - "github.com/authzed/authzed-go/pkg/responsemeta" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" "github.com/jzelinskie/stringz" "google.golang.org/grpc/codes" @@ -106,14 +105,6 @@ func (ps *permissionServer) CheckPermission(ctx context.Context, req *v1.CheckPe return nil, ps.rewriteError(ctx, cerr) } debugTrace = converted - - // TODO(jschorr): Remove this once the trailer is not longer used by anyone. - serr := responsemeta.SetResponseTrailerMetadata(ctx, map[responsemeta.ResponseMetadataTrailerKey]string{ - responsemeta.DebugInformation: `{"note": "debug information has been moved into the response object. See https://spicedb.dev/d/check-traces"}`, - }) - if serr != nil { - return nil, ps.rewriteError(ctx, serr) - } } if err != nil { diff --git a/internal/services/v1/permissions_test.go b/internal/services/v1/permissions_test.go index 031001f578..bd30cffbd0 100644 --- a/internal/services/v1/permissions_test.go +++ b/internal/services/v1/permissions_test.go @@ -293,7 +293,7 @@ func TestCheckPermissions(t *testing.T) { require.NoError(err) if debug { - require.NotNil(encodedDebugInfo) + require.Nil(encodedDebugInfo) debugInfo := checkResp.DebugTrace require.NotNil(debugInfo.Check) @@ -342,7 +342,8 @@ func TestCheckPermissionWithDebugInfo(t *testing.T) { encodedDebugInfo, err := responsemeta.GetResponseTrailerMetadataOrNil(trailer, responsemeta.DebugInformation) require.NoError(err) - require.NotNil(encodedDebugInfo) + // debug info is returned empty to make sure clients are not broken with backward incompatible payloads + require.Nil(encodedDebugInfo) debugInfo := checkResp.DebugTrace require.GreaterOrEqual(len(debugInfo.Check.GetSubProblems().Traces), 1)