Skip to content

Commit ed5fa89

Browse files
Merge pull request #291 from dropbox/codex/json-error-details
Add structured JSON error details
2 parents cbd4e5f + 69e89e1 commit ed5fa89

11 files changed

Lines changed: 221 additions & 29 deletions

File tree

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,8 @@ JSON error responses use stable `error.code` values:
191191
| `unknown_flag` | Cobra could not resolve a flag. |
192192
| `command_failed` | Fallback for failures without a more specific stable code. |
193193

194+
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`.
195+
194196
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:
195197

196198
```json

cmd/auth.go

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,11 @@ func getAccessToken(tokType string, domain string, force bool) (string, string,
238238
if !force && credential.shouldRefresh(time.Now()) {
239239
credential, err = refreshStoredCredential(tokType, domain, credential)
240240
if err != nil {
241+
details := authTokenDetails(tokType)
241242
if jsonErrorCode(err) == jsonErrorCodeAppKeyRequired {
242-
return "", "", appKeyRequiredErrorf("refresh saved Dropbox credentials: %w; run %q again", err, loginCommand(tokType))
243+
return "", "", appKeyRequiredErrorfWithDetails("refresh saved Dropbox credentials: %w; run %q again", details, err, loginCommand(tokType))
243244
}
244-
return "", "", authRefreshFailedErrorf("refresh saved Dropbox credentials: %w; run %q again", err, loginCommand(tokType))
245+
return "", "", authRefreshFailedErrorfWithDetails("refresh saved Dropbox credentials: %w; run %q again", details, err, loginCommand(tokType))
245246
}
246247
tokens[tokType] = credential
247248
if err = writeTokens(filePath, tokenMap); err != nil {
@@ -264,7 +265,26 @@ func loginCommand(tokType string) string {
264265
}
265266

266267
func missingAccessTokenError(tokType string) error {
267-
return authRequiredErrorf("no saved Dropbox credentials; run %q first or set %s", loginCommand(tokType), envAccessToken)
268+
return authRequiredErrorfWithDetails("no saved Dropbox credentials; run %q first or set %s", authTokenDetails(tokType), loginCommand(tokType), envAccessToken)
269+
}
270+
271+
func authTokenDetails(tokType string) map[string]any {
272+
return map[string]any{
273+
"token_type": authTokenTypeName(tokType),
274+
"login_command": loginCommand(tokType),
275+
"env_var": envAccessToken,
276+
}
277+
}
278+
279+
func authTokenTypeName(tokType string) string {
280+
switch tokType {
281+
case tokenTeamAccess:
282+
return "team-access"
283+
case tokenTeamManage:
284+
return "team-manage"
285+
default:
286+
return "personal"
287+
}
268288
}
269289

270290
func appCredentialsName(tokType string) string {
@@ -289,7 +309,9 @@ func ensureOAuthAppCredentials(tokType string) error {
289309
}
290310
creds.Key = strings.TrimSpace(creds.Key)
291311
if creds.Key == "" {
292-
return appKeyRequiredError("Dropbox app key is required")
312+
return appKeyRequiredErrorWithDetails("Dropbox app key is required", map[string]any{
313+
"token_type": authTokenTypeName(tokType),
314+
})
293315
}
294316

295317
setOAuthCredentials(tokType, creds.Key)
@@ -327,13 +349,19 @@ func requestAccessCredential(tokType string, domain string) (storedCredential, e
327349
}
328350
token, err := exchangeAuthorizationCode(context.Background(), conf, code, verifier)
329351
if err != nil {
330-
return storedCredential{}, authExchangeFailedErrorf("exchange authorization code: %w", err)
352+
return storedCredential{}, authExchangeFailedErrorfWithDetails("exchange authorization code: %w", map[string]any{
353+
"token_type": authTokenTypeName(tokType),
354+
}, err)
331355
}
332356
if token == nil || token.AccessToken == "" {
333-
return storedCredential{}, authExchangeFailedError("authorization did not return an access token")
357+
return storedCredential{}, authExchangeFailedErrorWithDetails("authorization did not return an access token", map[string]any{
358+
"token_type": authTokenTypeName(tokType),
359+
})
334360
}
335361
if token.RefreshToken == "" {
336-
return storedCredential{}, authExchangeFailedError("authorization did not return a refresh token")
362+
return storedCredential{}, authExchangeFailedErrorWithDetails("authorization did not return a refresh token", map[string]any{
363+
"token_type": authTokenTypeName(tokType),
364+
})
337365
}
338366
return storedCredentialFromOAuthToken(token, conf.ClientID), nil
339367
}
@@ -344,15 +372,19 @@ func refreshStoredCredential(tokType string, domain string, credential storedCre
344372
appKey = oauthCredentials(tokType)
345373
}
346374
if strings.TrimSpace(appKey) == "" {
347-
return storedCredential{}, appKeyRequiredError("saved credentials cannot be refreshed without a Dropbox app key")
375+
return storedCredential{}, appKeyRequiredErrorWithDetails("saved credentials cannot be refreshed without a Dropbox app key", map[string]any{
376+
"token_type": authTokenTypeName(tokType),
377+
})
348378
}
349379

350380
token, err := refreshOAuthToken(context.Background(), oauthConfigWithAppKey(appKey, domain), credential.oauthToken())
351381
if err != nil {
352382
return storedCredential{}, err
353383
}
354384
if token == nil || token.AccessToken == "" {
355-
return storedCredential{}, authRefreshFailedErrorf("token refresh did not return an access token")
385+
return storedCredential{}, authRefreshFailedErrorfWithDetails("token refresh did not return an access token", map[string]any{
386+
"token_type": authTokenTypeName(tokType),
387+
})
356388
}
357389

358390
refreshed := storedCredentialFromOAuthToken(token, appKey)

cmd/get.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ func ensureLocalDirectoryResult(source, target string, metadata files.IsMetadata
345345
status := getStatusCreated
346346
if info, err := os.Stat(target); err == nil {
347347
if !info.IsDir() {
348-
return getResult{}, pathConflictErrorf("path exists and is not a folder: %s", target)
348+
return getResult{}, pathConflictErrorWithPath(target, "path exists and is not a folder: %s", target)
349349
}
350350
status = getStatusExisting
351351
} else if !os.IsNotExist(err) {

cmd/json_contract_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,9 @@ func TestPublicJSONSchemaFiles(t *testing.T) {
192192
errorSchema := schema.Properties["error"]
193193
codeSchema := errorSchema.Properties["code"]
194194
assertStringSliceEqual(t, tt.file+" error code enum", codeSchema.Enum, expectedJSONErrorCodes())
195+
if _, ok := errorSchema.Properties["details"]; !ok {
196+
t.Fatalf("%s error schema missing details property", tt.file)
197+
}
195198
}
196199
})
197200
}

cmd/json_output.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ type jsonErrorResponse struct {
1515
}
1616

1717
type jsonError struct {
18-
Message string `json:"message"`
19-
Code string `json:"code"`
18+
Message string `json:"message"`
19+
Code string `json:"code"`
20+
Details map[string]any `json:"details,omitempty"`
2021
}
2122

2223
type jsonWarning struct {
@@ -56,6 +57,7 @@ func newJSONErrorResponse(cmd *cobra.Command, err error) jsonErrorResponse {
5657
Error: jsonError{
5758
Message: err.Error(),
5859
Code: jsonErrorCode(err),
60+
Details: jsonErrorDetails(err),
5961
},
6062
Warnings: emptyJSONWarnings(),
6163
}

cmd/mkdir.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func mkdir(cmd *cobra.Command, args []string) (err error) {
7676
}
7777
status = mkdirStatusExisting
7878
case ok && (conflictTag == files.WriteConflictErrorFile || conflictTag == files.WriteConflictErrorFileAncestor):
79-
return pathConflictErrorf("path exists and is not a folder: %s", dst)
79+
return pathConflictErrorWithPath(dst, "path exists and is not a folder: %s", dst)
8080
case ok:
8181
return err
8282
case isConflictError(err):
@@ -112,7 +112,7 @@ func existingFolderMetadata(dbx files.Client, dst string) (*files.FolderMetadata
112112
}
113113
folder, ok := metadata.(*files.FolderMetadata)
114114
if !ok || folder == nil {
115-
return nil, pathConflictErrorf("path exists and is not a folder: %s", dst)
115+
return nil, pathConflictErrorWithPath(dst, "path exists and is not a folder: %s", dst)
116116
}
117117
return folder, nil
118118
}

cmd/output.go

Lines changed: 88 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,15 @@ type jsonCodedError interface {
3737
JSONErrorCode() string
3838
}
3939

40+
type jsonDetailedError interface {
41+
error
42+
JSONErrorDetails() map[string]any
43+
}
44+
4045
type codedError struct {
41-
code string
42-
err error
46+
code string
47+
err error
48+
details map[string]any
4349
}
4450

4551
func (e codedError) Error() string {
@@ -54,13 +60,18 @@ func (e codedError) JSONErrorCode() string {
5460
return e.code
5561
}
5662

57-
func newCodedError(code string, err error) error {
63+
func (e codedError) JSONErrorDetails() map[string]any {
64+
return cloneJSONErrorDetails(e.details)
65+
}
66+
67+
func newCodedError(code string, err error, details ...map[string]any) error {
5868
if err == nil {
5969
return nil
6070
}
6171
return codedError{
62-
code: code,
63-
err: err,
72+
code: code,
73+
err: err,
74+
details: mergeJSONErrorDetails(details...),
6475
}
6576
}
6677

@@ -72,34 +83,52 @@ func invalidArgumentsErrorf(format string, args ...any) error {
7283
return newCodedError(jsonErrorCodeInvalidArguments, fmt.Errorf(format, args...))
7384
}
7485

75-
func pathConflictErrorf(format string, args ...any) error {
76-
return newCodedError(jsonErrorCodePathConflict, fmt.Errorf(format, args...))
86+
func pathConflictErrorWithPath(path string, format string, args ...any) error {
87+
return newCodedError(jsonErrorCodePathConflict, fmt.Errorf(format, args...), map[string]any{
88+
"path": path,
89+
})
7790
}
7891

7992
func authRequiredErrorf(format string, args ...any) error {
8093
return newCodedError(jsonErrorCodeAuthRequired, fmt.Errorf(format, args...))
8194
}
8295

96+
func authRequiredErrorfWithDetails(format string, details map[string]any, args ...any) error {
97+
return newCodedError(jsonErrorCodeAuthRequired, fmt.Errorf(format, args...), details)
98+
}
99+
83100
func appKeyRequiredError(message string) error {
84101
return newCodedError(jsonErrorCodeAppKeyRequired, errors.New(message))
85102
}
86103

87-
func appKeyRequiredErrorf(format string, args ...any) error {
88-
return newCodedError(jsonErrorCodeAppKeyRequired, fmt.Errorf(format, args...))
104+
func appKeyRequiredErrorWithDetails(message string, details map[string]any) error {
105+
return newCodedError(jsonErrorCodeAppKeyRequired, errors.New(message), details)
106+
}
107+
108+
func appKeyRequiredErrorfWithDetails(format string, details map[string]any, args ...any) error {
109+
return newCodedError(jsonErrorCodeAppKeyRequired, fmt.Errorf(format, args...), details)
89110
}
90111

91112
func authExchangeFailedError(message string) error {
92113
return newCodedError(jsonErrorCodeAuthExchangeFailed, errors.New(message))
93114
}
94115

95-
func authExchangeFailedErrorf(format string, args ...any) error {
96-
return newCodedError(jsonErrorCodeAuthExchangeFailed, fmt.Errorf(format, args...))
116+
func authExchangeFailedErrorWithDetails(message string, details map[string]any) error {
117+
return newCodedError(jsonErrorCodeAuthExchangeFailed, errors.New(message), details)
118+
}
119+
120+
func authExchangeFailedErrorfWithDetails(format string, details map[string]any, args ...any) error {
121+
return newCodedError(jsonErrorCodeAuthExchangeFailed, fmt.Errorf(format, args...), details)
97122
}
98123

99124
func authRefreshFailedErrorf(format string, args ...any) error {
100125
return newCodedError(jsonErrorCodeAuthRefreshFailed, fmt.Errorf(format, args...))
101126
}
102127

128+
func authRefreshFailedErrorfWithDetails(format string, details map[string]any, args ...any) error {
129+
return newCodedError(jsonErrorCodeAuthRefreshFailed, fmt.Errorf(format, args...), details)
130+
}
131+
103132
func commandOutput(cmd *cobra.Command) *output.Renderer {
104133
if cmd == nil {
105134
return output.New(nil, nil, output.FormatText)
@@ -275,6 +304,54 @@ func jsonErrorCode(err error) string {
275304
}
276305
}
277306

307+
func jsonErrorDetails(err error) map[string]any {
308+
details := make(map[string]any)
309+
310+
var detailed jsonDetailedError
311+
if errors.As(err, &detailed) {
312+
for key, value := range detailed.JSONErrorDetails() {
313+
details[key] = value
314+
}
315+
}
316+
317+
if summary, ok := dropboxAPIErrorSummary(err); ok {
318+
details["api_summary"] = summary
319+
} else if summary, ok := dropboxAPISummaryFromMessage(err.Error()); ok {
320+
details["api_summary"] = summary
321+
}
322+
323+
if len(details) == 0 {
324+
return nil
325+
}
326+
return details
327+
}
328+
329+
func cloneJSONErrorDetails(details map[string]any) map[string]any {
330+
if len(details) == 0 {
331+
return nil
332+
}
333+
cloned := make(map[string]any, len(details))
334+
for key, value := range details {
335+
cloned[key] = value
336+
}
337+
return cloned
338+
}
339+
340+
func mergeJSONErrorDetails(details ...map[string]any) map[string]any {
341+
merged := make(map[string]any)
342+
for _, detail := range details {
343+
for key, value := range detail {
344+
if value != nil {
345+
merged[key] = value
346+
}
347+
}
348+
}
349+
if len(merged) == 0 {
350+
return nil
351+
}
352+
return merged
353+
}
354+
278355
func dropboxAPIJSONErrorCode(err error) string {
279356
var rateLimitErr dropboxauth.RateLimitAPIError
280357
var rateLimitErrPtr *dropboxauth.RateLimitAPIError

0 commit comments

Comments
 (0)