From 97b128d45b67e1ad54134c200b58987186e2df34 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 9 Jul 2026 22:09:43 +0000 Subject: [PATCH] Improve role reassign invalid ID guidance Co-authored-by: Mike Coutermarsh --- internal/cmd/role/reassign.go | 11 +++-- internal/cmd/role/reassign_test.go | 67 +++++++++++++++++++++++++++++ internal/planetscale/client.go | 36 ++++++++++++++++ internal/planetscale/client_test.go | 46 ++++++++++++++++++++ 4 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 internal/cmd/role/reassign_test.go diff --git a/internal/cmd/role/reassign.go b/internal/cmd/role/reassign.go index a326871e..c6e9b4c6 100644 --- a/internal/cmd/role/reassign.go +++ b/internal/cmd/role/reassign.go @@ -24,6 +24,11 @@ func ReassignCmd(ch *cmdutil.Helper) *cobra.Command { Use: "reassign ", Short: "Reassign objects owned by a role to another role", Args: cmdutil.RequiredArgs("database", "branch", "role-id"), + Example: ` # List roles and copy the source role's id value + pscale role list mydb main --org my-org + + # Reassign objects owned by role tq8hnwl1mlty to successor role pscale_api_stqrbvtoy367 + pscale role reassign mydb main tq8hnwl1mlty --successor pscale_api_stqrbvtoy367 --org my-org`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() database := args[0] @@ -89,8 +94,8 @@ func ReassignCmd(ch *cmdutil.Helper) *cobra.Command { return cmdutil.HandleNotFoundWithServiceTokenCheck( ctx, cmd, ch.Config, ch.Client, err, "delete_branch_password or delete_production_branch_password", - "role %s does not exist in branch %s of database %s (organization: %s)", - printer.BoldBlue(roleID), printer.BoldBlue(branch), printer.BoldBlue(database), printer.BoldBlue(ch.Config.Organization)) + "role %s does not exist in branch %s of database %s (organization: %s)\n\nRun `pscale role list %s %s --org %s` and use the value from the `id` column for . Do not use the generated Postgres username from the `username` column.", + printer.BoldBlue(roleID), printer.BoldBlue(branch), printer.BoldBlue(database), printer.BoldBlue(ch.Config.Organization), database, branch, ch.Config.Organization) default: return cmdutil.HandleError(err) } @@ -116,7 +121,7 @@ func ReassignCmd(ch *cmdutil.Helper) *cobra.Command { } cmd.Flags().BoolVar(&flags.force, "force", false, "Reassign objects without confirmation") - cmd.Flags().StringVar(&flags.successor, "successor", "", "Role to transfer ownership to (required)") + cmd.Flags().StringVar(&flags.successor, "successor", "", "Successor role to transfer ownership to (required)") cmd.MarkFlagRequired("successor") // nolint:errcheck return cmd diff --git a/internal/cmd/role/reassign_test.go b/internal/cmd/role/reassign_test.go new file mode 100644 index 00000000..2d4a59bd --- /dev/null +++ b/internal/cmd/role/reassign_test.go @@ -0,0 +1,67 @@ +package role + +import ( + "bytes" + "context" + "strings" + "testing" + + "github.com/planetscale/cli/internal/cmdutil" + "github.com/planetscale/cli/internal/config" + "github.com/planetscale/cli/internal/mock" + ps "github.com/planetscale/cli/internal/planetscale" + "github.com/planetscale/cli/internal/printer" + + qt "github.com/frankban/quicktest" +) + +func TestRole_ReassignCmd_NotFoundGuidance(t *testing.T) { + c := qt.New(t) + + var buf bytes.Buffer + format := printer.Human + p := printer.NewPrinter(&format) + p.SetHumanOutput(&buf) + + org := "profound" + db := "platform" + branch := "dev" + roleID := "pscale_api_45lwf7v03mvy.ua0vjkcqtfid" + successor := "pscale_api_tnj3ssv8qulb" + + svc := &mock.PostgresRolesService{ + ReassignObjectsFn: func(ctx context.Context, req *ps.ReassignPostgresRoleObjectsRequest) error { + c.Assert(req.Organization, qt.Equals, org) + c.Assert(req.Database, qt.Equals, db) + c.Assert(req.Branch, qt.Equals, branch) + c.Assert(req.RoleId, qt.Equals, roleID) + c.Assert(req.Successor, qt.Equals, successor) + + return &ps.Error{Code: ps.ErrNotFound} + }, + } + + ch := &cmdutil.Helper{ + Printer: p, + Config: &config.Config{ + Organization: org, + }, + Client: func() (*ps.Client, error) { + return &ps.Client{ + PostgresRoles: svc, + }, nil + }, + } + + cmd := ReassignCmd(ch) + cmd.SetArgs([]string{db, branch, roleID, "--successor", successor, "--force"}) + err := cmd.Execute() + + c.Assert(err, qt.Not(qt.IsNil)) + c.Assert(svc.ReassignObjectsFnInvoked, qt.IsTrue) + + msg := err.Error() + c.Assert(strings.Contains(msg, "pscale role list platform dev --org profound"), qt.IsTrue) + c.Assert(strings.Contains(msg, "`id` column"), qt.IsTrue) + c.Assert(strings.Contains(msg, "`username` column"), qt.IsTrue) +} diff --git a/internal/planetscale/client.go b/internal/planetscale/client.go index 598aef56..fce224f3 100644 --- a/internal/planetscale/client.go +++ b/internal/planetscale/client.go @@ -349,11 +349,24 @@ func (c *Client) handleResponse(ctx context.Context, res *http.Response, v inter Message string `json:"message"` } + statusErrCode := errorCodeForHTTPStatus(res.StatusCode) errorRes := &errorResponse{} err = json.Unmarshal(out, errorRes) if err != nil { var jsonErr *json.SyntaxError if errors.As(err, &jsonErr) { + if statusErrCode != "" { + return &Error{ + msg: http.StatusText(res.StatusCode), + Code: statusErrCode, + Meta: map[string]string{ + "body": string(out), + "err": jsonErr.Error(), + "http_status": http.StatusText(res.StatusCode), + }, + } + } + return &Error{ msg: "malformed error response body received", Code: ErrResponseMalformed, @@ -374,6 +387,17 @@ func (c *Client) handleResponse(ctx context.Context, res *http.Response, v inter // they can debug the issue. // TODO(fatih): fix the behavior on the API side if *errorRes == (errorResponse{}) { + if statusErrCode != "" { + return &Error{ + msg: http.StatusText(res.StatusCode), + Code: statusErrCode, + Meta: map[string]string{ + "body": string(out), + "http_status": http.StatusText(res.StatusCode), + }, + } + } + return &Error{ msg: "internal error, response body doesn't match error type signature", Code: ErrInternal, @@ -395,6 +419,9 @@ func (c *Client) handleResponse(ctx context.Context, res *http.Response, v inter case "unprocessable": errCode = ErrRetry } + if errCode == "" { + errCode = statusErrCode + } return &Error{ msg: errorRes.Message, @@ -497,6 +524,15 @@ type serviceTokenTransport struct { tokenName string } +func errorCodeForHTTPStatus(statusCode int) ErrorCode { + switch statusCode { + case http.StatusNotFound: + return ErrNotFound + default: + return "" + } +} + func (t *serviceTokenTransport) RoundTrip(req *http.Request) (*http.Response, error) { req.Header.Add("Authorization", t.tokenName+":"+t.token) return t.rt.RoundTrip(req) diff --git a/internal/planetscale/client_test.go b/internal/planetscale/client_test.go index fc1315c8..763dfd0a 100644 --- a/internal/planetscale/client_test.go +++ b/internal/planetscale/client_test.go @@ -176,6 +176,52 @@ func TestDo(t *testing.T) { } } +func TestHandleResponse_UsesStatusCodeForInvalidNotFoundBody(t *testing.T) { + tests := []struct { + desc string + body string + }{ + { + desc: "empty object", + body: `{}`, + }, + { + desc: "malformed json", + body: `not-json`, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + c := qt.New(t) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, err := w.Write([]byte(tt.body)) + c.Assert(err, qt.IsNil) + })) + t.Cleanup(ts.Close) + + client, err := NewClient(WithBaseURL(ts.URL)) + c.Assert(err, qt.IsNil) + + req, err := client.newRequest(http.MethodGet, "/api-endpoint", nil) + c.Assert(err, qt.IsNil) + + res, err := client.client.Do(req) + c.Assert(err, qt.IsNil) + defer res.Body.Close() + + err = client.handleResponse(context.Background(), res, nil) + c.Assert(err, qt.Not(qt.IsNil)) + + perr, ok := err.(*Error) + c.Assert(ok, qt.IsTrue) + c.Assert(perr.Code, qt.Equals, ErrNotFound) + c.Assert(perr.Error(), qt.Equals, http.StatusText(http.StatusNotFound)) + }) + } +} + func Pointer[K any](val K) *K { return &val }