Add coded errors, JSON warnings, and contract tests

Replace fragile string-matching in jsonErrorCode with typed coded
errors set at the point of creation. Add warnings infrastructure
for deprecated commands and skipped symlinks during recursive upload.
Add contract tests that audit the structured output surface and
verify the JSON operation output envelope shape.

Author: AndreyVMarkelov

README.md

@@ -168,6 +168,7 @@ $ dbxcli restore --output=json /Reports/old.pdf 015f...
 Structured success output is rolling out command by command. Currently migrated commands are `version`, `account`, `du`, `ls`, `search`, `revs`, `cp`, `mv`, `put`, `get`, `share-link create`, `share-link list`, `share-link info`, `share-link update`, `share-link revoke`, `share-link download`, `share list folder`, `team info`, `team list-members`, `team list-groups`, `team add-member`, `team remove-member`, `mkdir`, `rm`, and `restore`. Commands that have not been migrated return a JSON error whose `error.message` is `structured output is not supported for this command yet` when used with `--output=json`.
 
 Command results and JSON errors are written to stdout. Status, progress, human-facing warnings, diagnostics, and verbose logs are written to stderr. JSON errors include a `warnings` array for machine-actionable warnings; it is `[]` when no warnings are present. Successful JSON payloads use the same `warnings` field.
+Current warning codes include `deprecated_command` for deprecated command paths and `skipped_symlink` for symlinks skipped by recursive upload.
 
 Successful JSON responses for migrated commands return an `input` object, a `results` array, and a `warnings` array. Result payloads are command-specific. For commands such as `mkdir`, each result reports what happened to the requested path:
 

cmd/account.go

@@ -15,7 +15,6 @@
 package cmd
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"text/tabwriter"
@@ -102,7 +101,7 @@ func renderBasicAccount(out io.Writer, ba *users.BasicAccount) error {
 
 func account(cmd *cobra.Command, args []string) error {
 	if len(args) > 1 {
-		return errors.New("`account` accepts an optional `id` argument")
+		return invalidArgumentsError("`account` accepts an optional `id` argument")
 	}
 
 	dbx := usersNewFunc(config)

cmd/add-member.go

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

cmd/cp.go

@@ -15,7 +15,6 @@
 package cmd
 
 import (
-	"errors"
 	"fmt"
 	"strings"
 
@@ -34,7 +33,7 @@ func cp(cmd *cobra.Command, args []string) error {
 		destination = args[1]
 		argsToCopy = append(argsToCopy, args[0])
 	} else {
-		return errors.New("cp requires a source and a destination")
+		return invalidArgumentsError("cp requires a source and a 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 errors.New("`get` requires `src` and/or `dst` arguments")
+		return invalidArgumentsError("`get` requires `src` and/or `dst` arguments")
 	}
 
 	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 errors.New("`get --output=json` cannot be used with stdout target `-`")
+			return invalidArgumentsError("`get --output=json` cannot be used with stdout target `-`")
 		}
 		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 fmt.Errorf("%s is a folder (use --recursive to download folders)", src)
+			return invalidArgumentsErrorf("%s is a folder (use --recursive to download folders)", src)
 		}
 		if f, statErr := os.Stat(dst); statErr == nil && f.IsDir() {
 			dst = filepath.Join(dst, path.Base(src))
@@ -196,15 +196,15 @@ func getOperationResults(results []getResult) []jsonOperationResult {
 
 func getStdout(cmd *cobra.Command, src string, recursive bool) error {
 	if recursive {
-		return errors.New("`get -` cannot be used with --recursive")
+		return invalidArgumentsError("`get -` cannot be used with --recursive")
 	}
 
 	dbx := filesNewFunc(config)
 
 	meta, err := dbx.GetMetadata(files.NewGetMetadataArg(src))
 	if err == nil {
 		if _, ok := meta.(*files.FolderMetadata); ok {
-			return fmt.Errorf("%s is a folder; cannot download folder to stdout", src)
+			return invalidArgumentsErrorf("%s is a folder; cannot download folder to stdout", src)
 		}
 	}
 
@@ -213,7 +213,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 errors.New("`get` requires `src` and/or `dst` arguments")
+		return invalidArgumentsError("`get` requires `src` and/or `dst` arguments")
 	}
 
 	src, err := validatePath(args[0])
@@ -342,7 +342,7 @@ func ensureLocalDirectoryResult(source, target string, metadata files.IsMetadata
 	status := getStatusCreated
 	if info, err := os.Stat(target); err == nil {
 		if !info.IsDir() {
-			return getResult{}, fmt.Errorf("path exists and is not a folder: %s", target)
+			return getResult{}, pathConflictErrorf("path exists and is not a folder: %s", target)
 		}
 		status = getStatusExisting
 	} else if !os.IsNotExist(err) {

cmd/get_test.go

@@ -509,6 +509,9 @@ func TestGetFolderWithoutRecursiveFlag(t *testing.T) {
 	if !strings.Contains(err.Error(), "--recursive") {
 		t.Errorf("error = %q, want mention of --recursive", err.Error())
 	}
+	if got, want := jsonErrorCode(err), jsonErrorCodeInvalidArguments; got != want {
+		t.Fatalf("jsonErrorCode = %q, want %q", got, want)
+	}
 }
 
 func TestGetRecursiveCommandGetsMetadataThenListsFolder(t *testing.T) {
@@ -718,11 +721,34 @@ func TestGetJSONRejectsStdoutTarget(t *testing.T) {
 	if !strings.Contains(err.Error(), "stdout target") {
 		t.Fatalf("error = %q, want stdout target", err.Error())
 	}
+	if got, want := jsonErrorCode(err), jsonErrorCodeInvalidArguments; got != want {
+		t.Fatalf("jsonErrorCode = %q, want %q", got, want)
+	}
 	if stdout.Len() != 0 {
 		t.Fatalf("stdout = %q, want empty", stdout.String())
 	}
 }
 
+func TestGetStdoutFolderErrorHasInvalidArgumentsCode(t *testing.T) {
+	mock := &mockFilesClient{
+		getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) {
+			return getTestFolderMetadata(arg.Path), nil
+		},
+	}
+	stubFilesClient(t, mock)
+
+	err := getStdout(&cobra.Command{}, "/remote-folder", false)
+	if err == nil {
+		t.Fatal("expected folder stdout error")
+	}
+	if !strings.Contains(err.Error(), "cannot download folder to stdout") {
+		t.Fatalf("error = %q, want stdout folder error", err.Error())
+	}
+	if got, want := jsonErrorCode(err), jsonErrorCodeInvalidArguments; got != want {
+		t.Fatalf("jsonErrorCode = %q, want %q", got, want)
+	}
+}
+
 func TestGetTextStdoutTargetWritesOnlyFileBytes(t *testing.T) {
 	mock := &mockFilesClient{
 		getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) {

cmd/json_contract_test.go

@@ -0,0 +1,130 @@
+package cmd
+
+import (
+	"bytes"
+	"encoding/json"
+	"errors"
+	"sort"
+	"strings"
+	"testing"
+
+	"github.com/dropbox/dbxcli/internal/output"
+	"github.com/spf13/cobra"
+)
+
+func TestStructuredOutputCommandAudit(t *testing.T) {
+	got := structuredOutputCommandPaths(RootCmd)
+	got = append(got, NewVersionCommand("test").Name())
+	sort.Strings(got)
+
+	want := []string{
+		"account",
+		"cp",
+		"du",
+		"get",
+		"ls",
+		"mkdir",
+		"mv",
+		"put",
+		"restore",
+		"revs",
+		"rm",
+		"search",
+		"share list folder",
+		"share list link",
+		"share-link create",
+		"share-link download",
+		"share-link info",
+		"share-link list",
+		"share-link revoke",
+		"share-link update",
+		"team add-member",
+		"team info",
+		"team list-groups",
+		"team list-members",
+		"team remove-member",
+		"version",
+	}
+	sort.Strings(want)
+
+	if len(got) != len(want) {
+		t.Fatalf("structured commands = %v, want %v", got, want)
+	}
+	for i := range want {
+		if got[i] != want[i] {
+			t.Fatalf("structured commands = %v, want %v", got, want)
+		}
+	}
+}
+
+func structuredOutputCommandPaths(root *cobra.Command) []string {
+	var paths []string
+	var walk func(*cobra.Command, []string)
+	walk = func(cmd *cobra.Command, parents []string) {
+		parts := parents
+		if cmd != root {
+			parts = append(append([]string{}, parents...), cmd.Name())
+			if commandSupportsStructuredOutput(cmd) {
+				paths = append(paths, strings.Join(parts, " "))
+			}
+		}
+		for _, child := range cmd.Commands() {
+			walk(child, parts)
+		}
+	}
+	walk(root, nil)
+	return paths
+}
+
+func TestJSONOperationOutputContractShape(t *testing.T) {
+	encoded, err := json.Marshal(newJSONOperationOutput(nil, nil, nil))
+	if err != nil {
+		t.Fatalf("marshal operation output: %v", err)
+	}
+
+	var raw map[string]json.RawMessage
+	if err := json.Unmarshal(encoded, &raw); err != nil {
+		t.Fatalf("decode operation output: %v", err)
+	}
+	for _, key := range []string{"input", "results", "warnings"} {
+		if _, ok := raw[key]; !ok {
+			t.Fatalf("operation output = %s, missing %q", encoded, key)
+		}
+	}
+	if len(raw) != 3 {
+		t.Fatalf("operation output = %s, want only input/results/warnings", encoded)
+	}
+}
+
+func TestUnsupportedCommandsReturnJSONErrorEnvelope(t *testing.T) {
+	for _, name := range []string{"login", "logout", "completion"} {
+		t.Run(name, func(t *testing.T) {
+			var stdout, stderr bytes.Buffer
+			cmd := &cobra.Command{Use: name}
+			cmd.SetOut(&stdout)
+			cmd.SetErr(&stderr)
+			cmd.Flags().String(outputFlag, "json", "")
+
+			err := validateOutputFormat(cmd)
+			if !errors.Is(err, output.ErrStructuredOutputUnsupported) {
+				t.Fatalf("validateOutputFormat error = %v, want structured output unsupported", err)
+			}
+
+			renderCommandError(cmd, err)
+			if stderr.Len() != 0 {
+				t.Fatalf("stderr = %q, want empty", stderr.String())
+			}
+
+			got := decodeJSONErrorResponse(t, stdout.String())
+			if got.OK {
+				t.Fatalf("ok = true, want false")
+			}
+			if got.Error.Code != jsonErrorCodeStructuredOutputUnsupported {
+				t.Fatalf("code = %q, want %q", got.Error.Code, jsonErrorCodeStructuredOutputUnsupported)
+			}
+			if got.Warnings == nil || len(got.Warnings) != 0 {
+				t.Fatalf("warnings = %+v, want empty array", got.Warnings)
+			}
+		})
+	}
+}

cmd/json_output.go

@@ -19,6 +19,11 @@ type jsonWarning struct {
 	Path    string `json:"path,omitempty"`
 }
 
+const (
+	jsonWarningCodeDeprecatedCommand = "deprecated_command"
+	jsonWarningCodeSkippedSymlink    = "skipped_symlink"
+)
+
 type jsonOperationOutput struct {
 	Input    any                   `json:"input"`
 	Results  []jsonOperationResult `json:"results"`
@@ -52,7 +57,11 @@ func newJSONOperationOutput(input any, results []jsonOperationResult, warnings [
 }
 
 func renderJSONOperationOutput(cmd *cobra.Command, input any, results []jsonOperationResult) error {
-	return commandOutput(cmd).Render(nil, newJSONOperationOutput(input, results, nil))
+	return renderJSONOperationOutputWithWarnings(cmd, input, results, nil)
+}
+
+func renderJSONOperationOutputWithWarnings(cmd *cobra.Command, input any, results []jsonOperationResult, warnings []jsonWarning) error {
+	return commandOutput(cmd).Render(nil, newJSONOperationOutput(input, results, warnings))
 }
 
 func newJSONOperationResult(status, kind string, input any, result any) jsonOperationResult {

cmd/mkdir.go

@@ -16,7 +16,6 @@ package cmd
 
 import (
 	"errors"
-	"fmt"
 	"strings"
 
 	"github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files"
@@ -44,7 +43,7 @@ type mkdirResult struct {
 
 func mkdir(cmd *cobra.Command, args []string) (err error) {
 	if len(args) != 1 {
-		return errors.New("`mkdir` requires a `directory` argument")
+		return invalidArgumentsError("`mkdir` requires a `directory` argument")
 	}
 
 	dst, err := validatePath(args[0])
@@ -77,7 +76,7 @@ func mkdir(cmd *cobra.Command, args []string) (err error) {
 			}
 			status = mkdirStatusExisting
 		case ok && (conflictTag == files.WriteConflictErrorFile || conflictTag == files.WriteConflictErrorFileAncestor):
-			return fmt.Errorf("path exists and is not a folder: %s", dst)
+			return pathConflictErrorf("path exists and is not a folder: %s", dst)
 		case ok:
 			return err
 		case isConflictError(err):
@@ -110,7 +109,7 @@ func existingFolderMetadata(dbx files.Client, dst string) (*files.FolderMetadata
 	}
 	folder, ok := metadata.(*files.FolderMetadata)
 	if !ok || folder == nil {
-		return nil, fmt.Errorf("path exists and is not a folder: %s", dst)
+		return nil, pathConflictErrorf("path exists and is not a folder: %s", dst)
 	}
 	return folder, nil
 }

cmd/mv.go

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