From a47904fdba58e8ca1f13ed61e6c8b5d93ec32803 Mon Sep 17 00:00:00 2001 From: Andrey Markelov Date: Tue, 30 Jun 2026 00:06:03 -0700 Subject: [PATCH] Add typed error details schema and deprecated-command warnings in errors Close error.schema.json details with additionalProperties: false and add typed properties for all known detail keys including bytes_written. Emit deprecated_command warning in JSON error responses for deprecated commands. Add live integration tests validating real command output against the public schemas. --- cmd/json_command_schema_test.go | 210 ++++++++++++++++++++++++++ cmd/json_output.go | 12 +- cmd/output_test.go | 28 ++++ cmd/share-list-links.go | 9 +- cmd/share_link_json_test.go | 7 +- docs/json-schema/v1/README.md | 11 +- docs/json-schema/v1/error.schema.json | 65 +++++++- 7 files changed, 327 insertions(+), 15 deletions(-) diff --git a/cmd/json_command_schema_test.go b/cmd/json_command_schema_test.go index 9510c60..d895607 100644 --- a/cmd/json_command_schema_test.go +++ b/cmd/json_command_schema_test.go @@ -18,12 +18,17 @@ import ( "bytes" "encoding/json" "errors" + "io" "os" + "path/filepath" "reflect" "testing" schemagen "github.com/dropbox/dbxcli/v3/internal/jsonschema" + "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files" + "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/sharing" "github.com/santhosh-tekuri/jsonschema/v6" + "github.com/spf13/cobra" ) func TestPublicJSONCommandSuccessSchemaMatchesGeneratedCatalog(t *testing.T) { @@ -110,6 +115,7 @@ func TestJSONErrorExamplesValidateAgainstPublicErrorSchema(t *testing.T) { {name: "invalid arguments", err: invalidArgumentsErrorfWithDetails("invalid --if-exists %q", flagValueErrorDetails("if-exists", "replace"), "replace")}, {name: "path conflict", err: pathConflictErrorWithPath("/file", "path exists: %s", "/file")}, {name: "auth required", err: missingAccessTokenError(tokenPersonal)}, + {name: "partial transfer", err: partialStdoutError(12)}, } for _, example := range examples { @@ -122,6 +128,173 @@ func TestJSONErrorExamplesValidateAgainstPublicErrorSchema(t *testing.T) { } } +func TestJSONErrorSchemaRejectsUnknownDetailsKey(t *testing.T) { + schema := compileJSONSchemaFile(t, "../docs/json-schema/v1/error.schema.json") + + value := normalizeJSONValueForSchema(t, newJSONErrorResponse(RootCmd, pathConflictErrorWithPath("/file", "path exists: %s", "/file"))) + details := jsonErrorDetailsFromSchemaValue(t, value) + details["unexpected"] = "value" + + if err := schema.Validate(value); err == nil { + t.Fatal("JSON error response with unknown details key validated successfully") + } +} + +func TestJSONErrorSchemaRejectsInvalidDetailsType(t *testing.T) { + schema := compileJSONSchemaFile(t, "../docs/json-schema/v1/error.schema.json") + + value := normalizeJSONValueForSchema(t, newJSONErrorResponse(RootCmd, partialStdoutError(12))) + details := jsonErrorDetailsFromSchemaValue(t, value) + details["bytes_written"] = "12" + + if err := schema.Validate(value); err == nil { + t.Fatal("JSON error response with invalid details type validated successfully") + } +} + +func TestLiveJSONSuccessOutputsValidateAgainstPublicCommandSuccessSchema(t *testing.T) { + schema := compileJSONSchemaFile(t, "../docs/json-schema/v1/commands.schema.json") + + t.Run("version", func(t *testing.T) { + var stdout bytes.Buffer + cmd := NewVersionCommand("test-version") + cmd.SetOut(&stdout) + cmd.Flags().String(outputFlag, "json", "") + + if err := versionCommand(cmd, "test-version"); err != nil { + t.Fatalf("versionCommand returned error: %v", err) + } + assertJSONBytesValidateAgainstSchema(t, schema, stdout.Bytes()) + }) + + t.Run("ls", func(t *testing.T) { + cmd, stdout := testLsCmd(t) + setLsOutputJSON(t, cmd) + stubFilesClient(t, &mockFilesClient{ + listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { + return &files.ListFolderResult{ + Entries: []files.IsMetadata{ + &files.FileMetadata{ + Metadata: files.Metadata{ + Name: "file.txt", + PathDisplay: "/file.txt", + PathLower: "/file.txt", + }, + Id: "id:file", + Rev: "rev-file", + Size: 42, + }, + }, + }, nil + }, + }) + + if err := ls(cmd, []string{"/"}); err != nil { + t.Fatalf("ls returned error: %v", err) + } + assertJSONBytesValidateAgainstSchema(t, schema, stdout.Bytes()) + }) + + t.Run("put", func(t *testing.T) { + tmpFile := writeJSONSchemaTempFile(t, "schema-live-put.txt", "data") + stubFilesClient(t, &mockFilesClient{ + uploadFn: func(arg *files.UploadArg, content io.Reader) (*files.FileMetadata, error) { + if _, err := io.ReadAll(content); err != nil { + t.Fatal(err) + } + return putFileMetadata(arg.Path, 4), nil + }, + }) + + var stdout bytes.Buffer + cmd := testPutJSONCmd(&stdout, nil) + cmd.Use = "put" + if err := put(cmd, []string{tmpFile, "/schema-live-put.txt"}); err != nil { + t.Fatalf("put returned error: %v", err) + } + assertJSONBytesValidateAgainstSchema(t, schema, stdout.Bytes()) + }) + + t.Run("share-link list", func(t *testing.T) { + stubSharedLinkClient(t, &mockSharedLinkClient{ + listSharedLinksFn: func(arg *sharing.ListSharedLinksArg) (*sharing.ListSharedLinksResult, error) { + return sharing.NewListSharedLinksResult([]sharing.IsSharedLinkMetadata{ + sharedLinkFile("/docs/report.txt", "https://example.com/report"), + }, false), nil + }, + }) + + var stdout bytes.Buffer + cmd := &cobra.Command{Use: "list"} + parent := &cobra.Command{Use: "share-link"} + root := &cobra.Command{Use: "dbxcli"} + root.AddCommand(parent) + parent.AddCommand(cmd) + cmd.SetOut(&stdout) + setShareLinkOutputJSON(t, cmd) + + if err := shareLinkList(cmd, []string{"/docs/report.txt"}); err != nil { + t.Fatalf("shareLinkList returned error: %v", err) + } + assertJSONBytesValidateAgainstSchema(t, schema, stdout.Bytes()) + }) +} + +func TestLiveJSONErrorOutputsValidateAgainstPublicErrorSchema(t *testing.T) { + schema := compileJSONSchemaFile(t, "../docs/json-schema/v1/error.schema.json") + + t.Run("invalid arguments", func(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + cmd := &cobra.Command{Use: "cp"} + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.Flags().String(outputFlag, "json", "") + + err := cp(cmd, []string{"/source"}) + if err == nil { + t.Fatal("cp returned nil error, want invalid arguments") + } + renderCommandError(cmd, err) + + if stderr.Len() != 0 { + t.Fatalf("stderr = %q, want empty", stderr.String()) + } + assertJSONBytesValidateAgainstSchema(t, schema, stdout.Bytes()) + }) + + t.Run("deprecated command error", func(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + cmd := &cobra.Command{ + Use: "link", + Deprecated: shareListLinksDeprecatedMessage, + } + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.Flags().String(outputFlag, "json", "") + + err := shareListLinks(cmd, []string{"/one", "/two"}) + if err == nil { + t.Fatal("shareListLinks returned nil error, want invalid arguments") + } + renderCommandError(cmd, err) + + if stderr.Len() != 0 { + t.Fatalf("stderr = %q, want empty", stderr.String()) + } + assertJSONBytesValidateAgainstSchema(t, schema, stdout.Bytes()) + + var got jsonErrorResponse + if err := json.Unmarshal(stdout.Bytes(), &got); err != nil { + t.Fatalf("decode JSON error response: %v", err) + } + if len(got.Warnings) != 1 || got.Warnings[0].Code != jsonWarningCodeDeprecatedCommand { + t.Fatalf("warnings = %+v, want deprecated command warning", got.Warnings) + } + }) +} + func loadCommandSchemaCatalog(t *testing.T) schemagen.CommandCatalog { t.Helper() @@ -193,6 +366,43 @@ func compileJSONSchemaFile(t *testing.T, file string) *jsonschema.Schema { return schema } +func assertJSONBytesValidateAgainstSchema(t *testing.T, schema *jsonschema.Schema, data []byte) { + t.Helper() + + value := decodeJSONValueForSchema(t, data) + if err := schema.Validate(value); err != nil { + t.Fatalf("JSON output does not validate against public schema: %v\noutput: %s", err, string(data)) + } +} + +func writeJSONSchemaTempFile(t *testing.T, name, content string) string { + t.Helper() + + path := filepath.Join(t.TempDir(), name) + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatalf("write temp file: %v", err) + } + return path +} + +func jsonErrorDetailsFromSchemaValue(t *testing.T, value any) map[string]any { + t.Helper() + + root, ok := value.(map[string]any) + if !ok { + t.Fatalf("value is %T, want object", value) + } + errorObject, ok := root["error"].(map[string]any) + if !ok { + t.Fatalf("error is %T, want object", root["error"]) + } + details, ok := errorObject["details"].(map[string]any) + if !ok { + t.Fatalf("details is %T, want object", errorObject["details"]) + } + return details +} + func decodeJSONValueForSchema(t *testing.T, data []byte) any { t.Helper() diff --git a/cmd/json_output.go b/cmd/json_output.go index 11a47ae..211aff3 100644 --- a/cmd/json_output.go +++ b/cmd/json_output.go @@ -60,7 +60,7 @@ func newJSONErrorResponse(cmd *cobra.Command, err error) jsonErrorResponse { Code: jsonErrorCode(err), Details: jsonErrorDetails(err), }, - Warnings: emptyJSONWarnings(), + Warnings: jsonCommandWarnings(cmd), } } @@ -148,6 +148,16 @@ func normalizeJSONWarnings(warnings []jsonWarning) []jsonWarning { return warnings } +func jsonCommandWarnings(cmd *cobra.Command) []jsonWarning { + if cmd == nil || cmd.Deprecated == "" { + return emptyJSONWarnings() + } + return []jsonWarning{{ + Code: jsonWarningCodeDeprecatedCommand, + Message: cmd.Deprecated, + }} +} + func jsonCommandPath(cmd *cobra.Command) string { if cmd == nil { return "" diff --git a/cmd/output_test.go b/cmd/output_test.go index b9f686c..c6559b1 100644 --- a/cmd/output_test.go +++ b/cmd/output_test.go @@ -319,6 +319,34 @@ func TestRenderCommandErrorWritesJSONErrorToStdout(t *testing.T) { } } +func TestRenderCommandErrorIncludesDeprecatedCommandWarning(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + cmd := &cobra.Command{ + Use: "link", + Deprecated: "use `dbxcli share-link list` instead", + } + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.Flags().String(outputFlag, "json", "") + + renderCommandError(cmd, errors.New("failed")) + + if got := stderr.String(); got != "" { + t.Fatalf("stderr = %q, want empty", got) + } + got := decodeJSONErrorResponse(t, stdout.String()) + if len(got.Warnings) != 1 { + t.Fatalf("warnings = %+v, want one warning", got.Warnings) + } + if got.Warnings[0].Code != jsonWarningCodeDeprecatedCommand { + t.Fatalf("warning code = %q, want %q", got.Warnings[0].Code, jsonWarningCodeDeprecatedCommand) + } + if got.Warnings[0].Message != "use `dbxcli share-link list` instead" { + t.Fatalf("warning message = %q, want deprecation message", got.Warnings[0].Message) + } +} + func TestRenderCommandErrorIncludesCodedDetails(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer diff --git a/cmd/share-list-links.go b/cmd/share-list-links.go index 2ac6158..dd65512 100644 --- a/cmd/share-list-links.go +++ b/cmd/share-list-links.go @@ -28,11 +28,10 @@ type shareLinkListInput struct { DirectOnly bool `json:"direct_only"` } +const shareListLinksDeprecatedMessage = "use `dbxcli share-link list` instead" + func shareListLinks(cmd *cobra.Command, args []string) (err error) { - return shareLinkListWithWarnings(cmd, args, []jsonWarning{{ - Code: jsonWarningCodeDeprecatedCommand, - Message: "use `dbxcli share-link list` instead", - }}) + return shareLinkListWithWarnings(cmd, args, jsonCommandWarnings(cmd)) } func shareLinkList(cmd *cobra.Command, args []string) error { @@ -154,7 +153,7 @@ When path is supplied, dbxcli lists direct shared links for that Dropbox path on var shareListLinksCmd = &cobra.Command{ Use: "link [path]", Short: "List shared links", - Deprecated: "use `dbxcli share-link list` instead", + Deprecated: shareListLinksDeprecatedMessage, RunE: shareListLinks, } diff --git a/cmd/share_link_json_test.go b/cmd/share_link_json_test.go index b9ae40f..a49b1a9 100644 --- a/cmd/share_link_json_test.go +++ b/cmd/share_link_json_test.go @@ -143,7 +143,7 @@ func TestDeprecatedShareListLinkJSONIncludesWarning(t *testing.T) { }) var stdout bytes.Buffer - cmd := &cobra.Command{} + cmd := &cobra.Command{Deprecated: shareListLinksDeprecatedMessage} cmd.SetOut(&stdout) setShareLinkOutputJSON(t, cmd) @@ -178,7 +178,10 @@ func TestDeprecatedShareListLinkJSONKeepsDeprecationTextOffStdout(t *testing.T) var stdout bytes.Buffer var stderr bytes.Buffer - cmd := &cobra.Command{} + cmd := &cobra.Command{ + Use: "link", + Deprecated: shareListLinksDeprecatedMessage, + } cmd.SetOut(&stdout) cmd.SetErr(&stderr) setShareLinkOutputJSON(t, cmd) diff --git a/docs/json-schema/v1/README.md b/docs/json-schema/v1/README.md index 871dafd..c97d3f6 100644 --- a/docs/json-schema/v1/README.md +++ b/docs/json-schema/v1/README.md @@ -43,7 +43,7 @@ Error responses always include: - `error.details`: optional machine-readable context, included only when dbxcli has reliable structured details such as `argument`, `arguments`, `flag`, `flags`, `value`, `path`, `token_type`, `login_command`, `env_var`, - Dropbox `api_summary`, or Dropbox `api_endpoint` + Dropbox `api_summary`, Dropbox `api_endpoint`, or `bytes_written` - `warnings`: machine-actionable warnings, or `[]` Command results and JSON errors are written to stdout. Status, progress, @@ -125,11 +125,12 @@ boolean; and `result.auth.auth_file` is `default`, `custom`, or `none`. dbxcli does not include the full auth file path by default. Warnings are objects with a stable `code` and human-readable `message`; they -may include optional command-specific details. Current warning codes include +may include optional command-specific details. JSON responses from deprecated +command paths include `deprecated_command`. Current warning codes include `deprecated_command` for deprecated command paths and `skipped_symlink` for -symlinks skipped by recursive upload. `logout` may return -`token_revoke_failed` when saved credentials were removed locally but one or -more Dropbox tokens could not be revoked remotely. +symlinks skipped by recursive upload. `logout` may return `token_revoke_failed` +when saved credentials were removed locally but one or more Dropbox tokens could +not be revoked remotely. Stable error codes: diff --git a/docs/json-schema/v1/error.schema.json b/docs/json-schema/v1/error.schema.json index 4edd34f..312a225 100644 --- a/docs/json-schema/v1/error.schema.json +++ b/docs/json-schema/v1/error.schema.json @@ -56,8 +56,69 @@ }, "details": { "type": "object", - "description": "Optional machine-readable context for stable error handling. Common keys include argument, arguments, flag, flags, value, path, token_type, login_command, env_var, api_summary, and api_endpoint. Present only when dbxcli has reliable structured details.", - "additionalProperties": true + "description": "Optional machine-readable context for stable error handling. Present only when dbxcli has reliable structured details.", + "additionalProperties": false, + "properties": { + "argument": { + "type": "string", + "description": "Single positional argument related to the error." + }, + "arguments": { + "type": "array", + "description": "Positional arguments related to the error.", + "items": { + "type": "string" + } + }, + "flag": { + "type": "string", + "description": "Single flag related to the error, without leading dashes." + }, + "flags": { + "type": "array", + "description": "Flags related to the error, without leading dashes.", + "items": { + "type": "string" + } + }, + "value": { + "type": [ + "string", + "number", + "boolean" + ], + "description": "Invalid or relevant value supplied by the caller." + }, + "path": { + "type": "string", + "description": "Local or Dropbox path related to the error." + }, + "token_type": { + "type": "string", + "description": "Credential type related to an auth error." + }, + "login_command": { + "type": "string", + "description": "Suggested login command for auth remediation." + }, + "env_var": { + "type": "string", + "description": "Environment variable related to auth or configuration remediation." + }, + "api_summary": { + "type": "string", + "description": "Dropbox API error summary when available." + }, + "api_endpoint": { + "type": "string", + "description": "Dropbox API endpoint parsed from an SDK error message when available." + }, + "bytes_written": { + "type": "integer", + "minimum": 0, + "description": "Number of bytes written before a partial stdout transfer failed." + } + } } } },