From a71572b7c246d05438dead1a37d289455e3649de Mon Sep 17 00:00:00 2001 From: Kevin Glynn Date: Tue, 7 Jul 2026 23:52:22 -0400 Subject: [PATCH] Fix unreachable dolt-login hint on PermissionDenied push errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hint text in handlePushError ("have you logged into DoltHub using 'dolt login'?") was unreachable due to two bugs: 1. The sentinel ErrUnknownPushErr was compared with switch/== but PushToRemoteBranch wraps it via fmt.Errorf("%w; %s", ...), so the equality check never matched. Fixed: use errors.Is. 2. The gRPC status was stringified (%s) in the same wrap, so status.FromError could never extract codes.PermissionDenied. Fixed: use %w to preserve the original error in the chain, and extract the status from RpcError via errors.As. Also fixes the RpcError type assertion (err.(*RpcError)) which suffered the same wrapping issue — now uses errors.As. Closes dolthub#783 Co-authored-by: Cursor --- go/cmd/dolt/commands/push.go | 27 +++++-- go/cmd/dolt/commands/push_test.go | 83 ++++++++++++++++++++ go/libraries/doltcore/env/actions/remotes.go | 2 +- 3 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 go/cmd/dolt/commands/push_test.go diff --git a/go/cmd/dolt/commands/push.go b/go/cmd/dolt/commands/push.go index aa0191a5d57..44fb8676808 100644 --- a/go/cmd/dolt/commands/push.go +++ b/go/cmd/dolt/commands/push.go @@ -26,7 +26,6 @@ import ( "github.com/gocraft/dbr/v2" "github.com/gocraft/dbr/v2/dialect" "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/errhand" @@ -197,25 +196,39 @@ func handlePushError(err error, usage cli.UsagePrinter) int { } var verr errhand.VerboseError - switch err { - case actions.ErrUnknownPushErr: - s, ok := status.FromError(err) - if ok && s.Code() == codes.PermissionDenied { + if errors.Is(err, actions.ErrUnknownPushErr) { + if isPermissionDeniedErr(err) { cli.Println("hint: have you logged into DoltHub using 'dolt login'?") cli.Println("hint: check that user.email in 'dolt config --list' has write perms to DoltHub repo") } - if rpcErr, ok := err.(*remotestorage.RpcError); ok { + var rpcErr *remotestorage.RpcError + if errors.As(err, &rpcErr) { verr = errhand.BuildDError("error: push failed").AddCause(err).AddDetails("%s", rpcErr.FullDetails()).Build() } else { verr = errhand.BuildDError("error: push failed").AddCause(err).Build() } - default: + } else { verr = errhand.VerboseErrorFromError(err) } return HandleVErrAndExitCode(verr, usage) } +// isPermissionDeniedErr reports whether err indicates a gRPC PermissionDenied +// failure. It first tries to extract an RpcError from the chain (which captures +// the gRPC status at creation time), then falls back to checking the error text +// for the status code name. +func isPermissionDeniedErr(err error) bool { + var rpcErr *remotestorage.RpcError + if errors.As(err, &rpcErr) { + st := remotestorage.GetStatus(rpcErr) + if st != nil && st.Code() == codes.PermissionDenied { + return true + } + } + return strings.Contains(err.Error(), "PermissionDenied") +} + // progLanguage is the language to use when displaying progress for a pull from a src db to a sink db. type progLanguage int diff --git a/go/cmd/dolt/commands/push_test.go b/go/cmd/dolt/commands/push_test.go new file mode 100644 index 00000000000..9e5b6d48be8 --- /dev/null +++ b/go/cmd/dolt/commands/push_test.go @@ -0,0 +1,83 @@ +// Copyright 2026 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package commands + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" + "github.com/dolthub/dolt/go/libraries/doltcore/remotestorage" +) + +func TestIsPermissionDeniedErr(t *testing.T) { + tests := []struct { + name string + err error + expected bool + }{ + { + name: "wrapped RpcError with PermissionDenied", + err: wrappedRpcError(codes.PermissionDenied, "access denied"), + expected: true, + }, + { + name: "wrapped RpcError with Internal", + err: wrappedRpcError(codes.Internal, "server error"), + expected: false, + }, + { + name: "wrapped RpcError with Unauthenticated", + err: wrappedRpcError(codes.Unauthenticated, "not authenticated"), + expected: false, + }, + { + name: "plain error with PermissionDenied in text", + err: fmt.Errorf("%w: rpc error: code = PermissionDenied desc = no access", actions.ErrUnknownPushErr), + expected: true, + }, + { + name: "plain error without PermissionDenied", + err: fmt.Errorf("%w: connection refused", actions.ErrUnknownPushErr), + expected: false, + }, + { + name: "nil error", + err: nil, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.err == nil { + assert.False(t, isPermissionDeniedErr(errors.New(""))) + return + } + assert.Equal(t, tt.expected, isPermissionDeniedErr(tt.err)) + }) + } +} + +func wrappedRpcError(code codes.Code, msg string) error { + grpcErr := status.Error(code, msg) + rpcErr := remotestorage.NewRpcError(grpcErr, "Push", "dolthub.com", nil) + return fmt.Errorf("%w: %w", actions.ErrUnknownPushErr, rpcErr) +} diff --git a/go/libraries/doltcore/env/actions/remotes.go b/go/libraries/doltcore/env/actions/remotes.go index 26cb9eafef2..19840ac017b 100644 --- a/go/libraries/doltcore/env/actions/remotes.go +++ b/go/libraries/doltcore/env/actions/remotes.go @@ -250,7 +250,7 @@ func PushToRemoteBranch[C doltdb.Context](ctx C, rsr env.RepoStateReader[C], tem case doltdb.ErrUpToDate, doltdb.ErrIsAhead, ErrCantFF, datas.ErrMergeNeeded, datas.ErrDirtyWorkspace, ErrShallowPushImpossible: return err default: - return fmt.Errorf("%w; %s", ErrUnknownPushErr, err.Error()) + return fmt.Errorf("%w: %w", ErrUnknownPushErr, err) } }