Merge pull request #292 from dropbox/codex/json-error-detail-producers

Add structured error details to all invalid_arguments errors

Author: AndreyVMarkelov

README.md

@@ -191,7 +191,7 @@ JSON error responses use stable `error.code` values:
 | `unknown_flag`                  | Cobra could not resolve a flag.                                                   |
 | `command_failed`                | Fallback for failures without a more specific stable code.                        |
 
-JSON errors may also include an optional `error.details` object when dbxcli has reliable machine-readable context. Current detail keys include `path` for known path conflicts; `token_type`, `login_command`, and `env_var` for auth remediation; and `api_summary` for Dropbox API errors. Generic local failures omit `error.details`.
+JSON errors may also include an optional `error.details` object when dbxcli has reliable machine-readable context. Current detail keys include `argument`, `arguments`, `flag`, `flags`, and `value` for dbxcli-owned validation errors; `path` for known path conflicts; `token_type`, `login_command`, and `env_var` for auth remediation; and `api_summary` and `api_endpoint` for Dropbox API errors. Generic local failures omit `error.details`.
 
 Successful JSON responses for migrated commands return `ok: true`, `schema_version: "1"`, `command`, an `input` object, a `results` array, and a `warnings` array. Result payloads are command-specific. Public top-level schemas and the command contract catalog live under [docs/json-schema/v1](docs/json-schema/v1/). If a multi-target or recursive command fails after some side effects have already happened, dbxcli returns a JSON error envelope and does not include partial success results. For commands such as `mkdir`, each result reports what happened to the requested path:
 

cmd/account.go

@@ -104,7 +104,7 @@ func renderBasicAccount(out io.Writer, ba *users.BasicAccount) error {
 
 func account(cmd *cobra.Command, args []string) error {
 	if len(args) > 1 {
-		return invalidArgumentsError("`account` accepts an optional `id` argument")
+		return invalidArgumentsErrorWithDetails("`account` accepts an optional `id` argument", argumentErrorDetails("id"))
 	}
 
 	dbx := usersNewFunc(config)

cmd/add-member.go

@@ -24,7 +24,7 @@ import (
 
 func addMember(cmd *cobra.Command, args []string) (err error) {
 	if len(args) != 3 {
-		return invalidArgumentsError("`add-member` requires `email`, `first`, and `last` arguments")
+		return invalidArgumentsErrorWithDetails("`add-member` requires `email`, `first`, and `last` arguments", argumentsErrorDetails("email", "first", "last"))
 	}
 	dbx := teamNewFunc(config)
 

cmd/cp.go

@@ -34,7 +34,7 @@ func cp(cmd *cobra.Command, args []string) error {
 		destination = args[1]
 		argsToCopy = append(argsToCopy, args[0])
 	} else {
-		return invalidArgumentsError("cp requires a source and a destination")
+		return invalidArgumentsErrorWithDetails("cp requires a source and a destination", argumentsErrorDetails("source", "destination"))
 	}
 
 	var cpErrors []error

cmd/get.go

@@ -65,7 +65,7 @@ type getResult struct {
 
 func get(cmd *cobra.Command, args []string) (err error) {
 	if len(args) == 0 || len(args) > 2 {
-		return invalidArgumentsError("`get` requires `src` and/or `dst` arguments")
+		return invalidArgumentsErrorWithDetails("`get` requires `src` and/or `dst` arguments", argumentsErrorDetails("src", "dst"))
 	}
 
 	src, err := validatePath(args[0])
@@ -83,7 +83,7 @@ func get(cmd *cobra.Command, args []string) (err error) {
 
 	if dst == "-" {
 		if commandOutputFormat(cmd) == output.FormatJSON {
-			return invalidArgumentsError("`get --output=json` cannot be used with stdout target `-`")
+			return invalidArgumentsErrorWithDetails("`get --output=json` cannot be used with stdout target `-`", mergeJSONErrorDetails(argumentErrorDetails("dst"), flagErrorDetails("output")))
 		}
 		return getStdout(cmd, src, recursive)
 	}
@@ -113,7 +113,7 @@ func get(cmd *cobra.Command, args []string) (err error) {
 
 	if _, ok := meta.(*files.FolderMetadata); ok {
 		if !recursive {
-			return invalidArgumentsErrorf("%s is a folder (use --recursive to download folders)", src)
+			return invalidArgumentsErrorfWithDetails("%s is a folder (use --recursive to download folders)", pathErrorDetails(src), src)
 		}
 		if f, statErr := os.Stat(dst); statErr == nil && f.IsDir() {
 			dst = filepath.Join(dst, path.Base(src))
@@ -199,15 +199,15 @@ func getOperationResults(results []getResult) []jsonOperationResult {
 
 func getStdout(cmd *cobra.Command, src string, recursive bool) error {
 	if recursive {
-		return invalidArgumentsError("`get -` cannot be used with --recursive")
+		return invalidArgumentsErrorWithDetails("`get -` cannot be used with --recursive", flagErrorDetails("recursive"))
 	}
 
 	dbx := filesNewFunc(config)
 
 	meta, err := dbx.GetMetadata(files.NewGetMetadataArg(src))
 	if err == nil {
 		if _, ok := meta.(*files.FolderMetadata); ok {
-			return invalidArgumentsErrorf("%s is a folder; cannot download folder to stdout", src)
+			return invalidArgumentsErrorfWithDetails("%s is a folder; cannot download folder to stdout", pathErrorDetails(src), src)
 		}
 	}
 
@@ -216,7 +216,7 @@ func getStdout(cmd *cobra.Command, src string, recursive bool) error {
 
 func getWithClient(dbx files.Client, args []string) (err error) {
 	if len(args) == 0 || len(args) > 2 {
-		return invalidArgumentsError("`get` requires `src` and/or `dst` arguments")
+		return invalidArgumentsErrorWithDetails("`get` requires `src` and/or `dst` arguments", argumentsErrorDetails("src", "dst"))
 	}
 
 	src, err := validatePath(args[0])

cmd/mkdir.go

@@ -43,7 +43,7 @@ type mkdirResult struct {
 
 func mkdir(cmd *cobra.Command, args []string) (err error) {
 	if len(args) != 1 {
-		return invalidArgumentsError("`mkdir` requires a `directory` argument")
+		return invalidArgumentsErrorWithDetails("`mkdir` requires a `directory` argument", argumentErrorDetails("directory"))
 	}
 
 	dst, err := validatePath(args[0])

cmd/mv.go

@@ -34,7 +34,7 @@ func mv(cmd *cobra.Command, args []string) error {
 		destination = args[1]
 		argsToMove = append(argsToMove, args[0])
 	} else {
-		return invalidArgumentsError("mv command requires a source and a destination")
+		return invalidArgumentsErrorWithDetails("mv command requires a source and a destination", argumentsErrorDetails("source", "destination"))
 	}
 
 	var mvErrors []error

cmd/output.go

@@ -75,18 +75,16 @@ func newCodedError(code string, err error, details ...map[string]any) error {
 	}
 }
 
-func invalidArgumentsError(message string) error {
-	return newCodedError(jsonErrorCodeInvalidArguments, errors.New(message))
+func invalidArgumentsErrorWithDetails(message string, details map[string]any) error {
+	return newCodedError(jsonErrorCodeInvalidArguments, errors.New(message), details)
 }
 
-func invalidArgumentsErrorf(format string, args ...any) error {
-	return newCodedError(jsonErrorCodeInvalidArguments, fmt.Errorf(format, args...))
+func invalidArgumentsErrorfWithDetails(format string, details map[string]any, args ...any) error {
+	return newCodedError(jsonErrorCodeInvalidArguments, fmt.Errorf(format, args...), details)
 }
 
 func pathConflictErrorWithPath(path string, format string, args ...any) error {
-	return newCodedError(jsonErrorCodePathConflict, fmt.Errorf(format, args...), map[string]any{
-		"path": path,
-	})
+	return newCodedError(jsonErrorCodePathConflict, fmt.Errorf(format, args...), pathErrorDetails(path))
 }
 
 func authRequiredErrorf(format string, args ...any) error {
@@ -129,6 +127,33 @@ func authRefreshFailedErrorfWithDetails(format string, details map[string]any, a
 	return newCodedError(jsonErrorCodeAuthRefreshFailed, fmt.Errorf(format, args...), details)
 }
 
+func argumentErrorDetails(argument string) map[string]any {
+	return map[string]any{"argument": argument}
+}
+
+func argumentsErrorDetails(arguments ...string) map[string]any {
+	return map[string]any{"arguments": arguments}
+}
+
+func flagErrorDetails(flag string) map[string]any {
+	return map[string]any{"flag": flag}
+}
+
+func flagsErrorDetails(flags ...string) map[string]any {
+	return map[string]any{"flags": flags}
+}
+
+func flagValueErrorDetails(flag, value string) map[string]any {
+	return map[string]any{
+		"flag":  flag,
+		"value": value,
+	}
+}
+
+func pathErrorDetails(path string) map[string]any {
+	return map[string]any{"path": path}
+}
+
 func commandOutput(cmd *cobra.Command) *output.Renderer {
 	if cmd == nil {
 		return output.New(nil, nil, output.FormatText)
@@ -319,6 +344,9 @@ func jsonErrorDetails(err error) map[string]any {
 	} else if summary, ok := dropboxAPISummaryFromMessage(err.Error()); ok {
 		details["api_summary"] = summary
 	}
+	if endpoint, ok := dropboxAPIEndpointFromMessage(err.Error()); ok {
+		details["api_endpoint"] = endpoint
+	}
 
 	if len(details) == 0 {
 		return nil
@@ -471,6 +499,24 @@ func dropboxAPISummaryFromMessage(message string) (string, bool) {
 	return "", false
 }
 
+func dropboxAPIEndpointFromMessage(message string) (string, bool) {
+	const prefix = `Error in call to API function "`
+	idx := strings.Index(message, prefix)
+	if idx < 0 {
+		return "", false
+	}
+	start := idx + len(prefix)
+	end := strings.Index(message[start:], `"`)
+	if end < 0 {
+		return "", false
+	}
+	end += start
+	if start == end {
+		return "", false
+	}
+	return message[start:end], true
+}
+
 func isDropboxAPISummary(message string) bool {
 	if message == "" || strings.ContainsAny(message, " \t\r\n\"") || !strings.Contains(message, "/") {
 		return false

cmd/output_test.go

@@ -341,6 +341,28 @@ func TestRenderCommandErrorIncludesCodedDetails(t *testing.T) {
 	}
 }
 
+func TestRenderCommandErrorIncludesArgumentAndFlagDetails(t *testing.T) {
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+	cmd := &cobra.Command{Use: "put"}
+	cmd.SetOut(&stdout)
+	cmd.SetErr(&stderr)
+	cmd.Flags().String(outputFlag, "json", "")
+
+	renderCommandError(cmd, invalidArgumentsErrorfWithDetails("invalid --if-exists %q (use overwrite, skip, or fail)", flagValueErrorDetails("if-exists", "replace"), "replace"))
+
+	if got := stderr.String(); got != "" {
+		t.Fatalf("stderr = %q, want empty", got)
+	}
+	got := decodeJSONErrorResponse(t, stdout.String())
+	if got.Error.Code != jsonErrorCodeInvalidArguments {
+		t.Fatalf("code = %q, want %q", got.Error.Code, jsonErrorCodeInvalidArguments)
+	}
+	if got.Error.Details["flag"] != "if-exists" || got.Error.Details["value"] != "replace" {
+		t.Fatalf("details = %+v, want flag/value", got.Error.Details)
+	}
+}
+
 func TestRenderCommandErrorIncludesDropboxAPISummaryDetails(t *testing.T) {
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
@@ -364,6 +386,32 @@ func TestRenderCommandErrorIncludesDropboxAPISummaryDetails(t *testing.T) {
 	}
 }
 
+func TestRenderCommandErrorIncludesDropboxAPIEndpointDetails(t *testing.T) {
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+	cmd := &cobra.Command{Use: "ls"}
+	cmd.SetOut(&stdout)
+	cmd.SetErr(&stderr)
+	cmd.Flags().String(outputFlag, "json", "")
+
+	err := errors.New(`Error in call to API function "files/list_folder": path/not_found/.`)
+	renderCommandError(cmd, err)
+
+	if got := stderr.String(); got != "" {
+		t.Fatalf("stderr = %q, want empty", got)
+	}
+	got := decodeJSONErrorResponse(t, stdout.String())
+	if got.Error.Code != jsonErrorCodeNotFound {
+		t.Fatalf("code = %q, want %q", got.Error.Code, jsonErrorCodeNotFound)
+	}
+	if got.Error.Details["api_endpoint"] != "files/list_folder" {
+		t.Fatalf("details = %+v, want api_endpoint", got.Error.Details)
+	}
+	if got.Error.Details["api_summary"] != `Error in call to API function "files/list_folder": path/not_found/.` {
+		t.Fatalf("details = %+v, want api_summary", got.Error.Details)
+	}
+}
+
 func TestJSONErrorDetailsIncludesAuthRemediation(t *testing.T) {
 	got := newJSONErrorResponse(&cobra.Command{Use: "account"}, missingAccessTokenError(tokenPersonal))
 
@@ -694,12 +742,12 @@ func TestJSONErrorCodeUsesCodedErrors(t *testing.T) {
 		},
 		{
 			name: "optional argument validation",
-			err:  invalidArgumentsError("`account` accepts an optional `id` argument"),
+			err:  invalidArgumentsErrorWithDetails("`account` accepts an optional `id` argument", argumentErrorDetails("id")),
 			want: jsonErrorCodeInvalidArguments,
 		},
 		{
 			name: "required argument validation",
-			err:  invalidArgumentsError("`add-member` requires `email`, `first`, and `last` arguments"),
+			err:  invalidArgumentsErrorWithDetails("`add-member` requires `email`, `first`, and `last` arguments", argumentsErrorDetails("email", "first", "last")),
 			want: jsonErrorCodeInvalidArguments,
 		},
 		{

cmd/put.go

@@ -284,7 +284,7 @@ func putOperationResults(results []putResult) []jsonOperationResult {
 
 func put(cmd *cobra.Command, args []string) (err error) {
 	if len(args) == 0 || len(args) > 2 {
-		return invalidArgumentsError("`put` requires `src` and/or `dst` arguments")
+		return invalidArgumentsErrorWithDetails("`put` requires `src` and/or `dst` arguments", argumentsErrorDetails("src", "dst"))
 	}
 
 	opts, err := parsePutOptions(cmd)
@@ -306,7 +306,7 @@ func put(cmd *cobra.Command, args []string) (err error) {
 	}
 
 	if srcInfo.IsDir() && !recursive {
-		return invalidArgumentsErrorf("%s is a directory (use --recursive to upload directories)", src)
+		return invalidArgumentsErrorfWithDetails("%s is a directory (use --recursive to upload directories)", pathErrorDetails(src), src)
 	}
 
 	// Default `dst` to the base segment of the source path; use the second argument if provided.
@@ -356,15 +356,15 @@ func put(cmd *cobra.Command, args []string) (err error) {
 
 func putStdin(cmd *cobra.Command, args []string, opts putOptions, recursive bool) error {
 	if len(args) < 2 {
-		return invalidArgumentsError("`put -` requires an explicit target path")
+		return invalidArgumentsErrorWithDetails("`put -` requires an explicit target path", argumentErrorDetails("dst"))
 	}
 	if recursive {
-		return invalidArgumentsError("`put -` cannot be used with --recursive")
+		return invalidArgumentsErrorWithDetails("`put -` cannot be used with --recursive", flagErrorDetails("recursive"))
 	}
 
 	dst := args[1]
 	if strings.HasSuffix(dst, "/") {
-		return invalidArgumentsErrorf("cannot upload stdin to directory target %q; provide a full Dropbox file path", dst)
+		return invalidArgumentsErrorfWithDetails("cannot upload stdin to directory target %q; provide a full Dropbox file path", pathErrorDetails(dst), dst)
 	}
 
 	dstPath, err := validatePath(dst)
@@ -446,7 +446,7 @@ func parsePutOptions(cmd *cobra.Command) (putOptions, error) {
 		return putOptions{}, err
 	}
 	if chunkSize%(1<<22) != 0 {
-		return putOptions{}, invalidArgumentsError("`put` requires chunk size to be multiple of 4MiB")
+		return putOptions{}, invalidArgumentsErrorWithDetails("`put` requires chunk size to be multiple of 4MiB", flagErrorDetails("chunksize"))
 	}
 	workers, err := cmd.Flags().GetInt("workers")
 	if err != nil {
@@ -486,7 +486,7 @@ func normalizePutIfExists(ifExists string) (string, error) {
 	case putIfExistsOverwrite, putIfExistsSkip, putIfExistsFail:
 		return ifExists, nil
 	default:
-		return "", invalidArgumentsErrorf("invalid --if-exists %q (use overwrite, skip, or fail)", ifExists)
+		return "", invalidArgumentsErrorfWithDetails("invalid --if-exists %q (use overwrite, skip, or fail)", flagValueErrorDetails("if-exists", ifExists), ifExists)
 	}
 }
 
@@ -578,7 +578,7 @@ func checkPutStdinDestination(dbx files.Client, dst string, ifExists string) (pu
 		return putDestinationUpload, nil, nil
 	}
 	if _, ok := meta.(*files.FolderMetadata); ok {
-		return putDestinationUpload, nil, invalidArgumentsErrorf("cannot upload stdin to folder %q; provide a full Dropbox file path", dst)
+		return putDestinationUpload, nil, invalidArgumentsErrorfWithDetails("cannot upload stdin to folder %q; provide a full Dropbox file path", pathErrorDetails(dst), dst)
 	}
 	return actionForExistingDestination(dst, ifExists, meta)
 }