Skip to content

Commit 5cf7e04

Browse files
committed
feat(drive): emit typed error envelopes across the drive domain
Drive-domain errors now leave the CLI as typed, machine-branchable envelopes — a stable `type` plus `subtype` and named fields (param, params, retryable, log_id) — so scripts and AI agents can branch on structure instead of parsing prose. Changes: - All drive error producers emit typed errs.* errors; the exit code is derived from the error category. - Upload/push API errors are classified through the shared typed classifier, surfacing subtype, log_id, and troubleshooter. - Duplicate rel_path conflicts report each colliding path as a structured params entry (RFC 7807 invalid-params style). - A golangci-lint guard locks the drive path so legacy error construction cannot be reintroduced — the template for the remaining domains. Output changes worth noting for consumers: - Error envelopes now carry typed type/subtype and named fields; exit codes follow the error category (malformed or incomplete API responses are reported as internal errors rather than generic API errors). - `+pull`/`+sync` partial failures print the per-item result to stdout (with a `note` field) and signal failure through the exit code, matching `+push`; the exit code itself is unchanged. Errors that still flow through shared helpers keep the legacy shape until the shared layer migrates in a later change.
1 parent 3bfb809 commit 5cf7e04

46 files changed

Lines changed: 1433 additions & 655 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.golangci.yml

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,23 @@ linters:
6565
- forbidigo
6666
# errs-typed-only enforced on paths already migrated to errs.NewXxxError.
6767
# Add a path when its migration is complete.
68-
- path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|cmd/auth/|cmd/config/|cmd/service/|shortcuts/common/mcp_client\.go|shortcuts/calendar/helpers\.go)
68+
- path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|cmd/auth/|cmd/config/|cmd/service/|shortcuts/common/mcp_client\.go|shortcuts/calendar/helpers\.go|shortcuts/drive/)
6969
text: errs-typed-only
7070
linters:
7171
- forbidigo
72+
# errs-no-bare-wrap enforced on paths fully migrated to typed final
73+
# errors. Scoped separately from errs-typed-only because cmd/auth/,
74+
# cmd/config/ still have residual fmt.Errorf and must not be caught.
75+
- path-except: (shortcuts/drive/|shortcuts/calendar/helpers\.go|shortcuts/common/mcp_client\.go)
76+
text: errs-no-bare-wrap
77+
linters:
78+
- forbidigo
79+
# errs-no-legacy-helper is drive-only: the shared helpers it bans are
80+
# still used by other domains until their later migration phase.
81+
- path-except: (shortcuts/drive/)
82+
text: errs-no-legacy-helper
83+
linters:
84+
- forbidigo
7285

7386
settings:
7487
depguard:
@@ -94,6 +107,23 @@ linters:
94107
msg: >-
95108
[errs-typed-only] use errs.NewXxxError(...) builder
96109
(see errs/types.go).
110+
# ── legacy shared error helpers banned on drive ──
111+
# These helpers internally produce legacy output.Err* shapes, so they
112+
# are invisible to the errs-typed-only ban above. Drive has migrated its
113+
# calls to typed errs.* (drive-local driveInputStatError / driveSaveError);
114+
# this prevents reintroduction. Other domains still use the shared
115+
# helpers (migrated globally in a later phase), so this is drive-scoped.
116+
- pattern: (common\.FlagErrorf|common\.WrapInputStatError|common\.WrapSaveErrorByCategory)\b
117+
msg: >-
118+
[errs-no-legacy-helper] these shared helpers emit legacy output.Err*
119+
shapes. Use the typed errs.NewXxxError builders or the drive-local
120+
driveInputStatError / driveSaveError helpers (shortcuts/drive/drive_errors.go).
121+
# ── bare error wraps banned on fully-typed paths ──
122+
- pattern: (fmt\.Errorf|errors\.New)\b
123+
msg: >-
124+
[errs-no-bare-wrap] final errors must be typed (errs.NewXxxError);
125+
wrap a cause with .WithCause(err). Genuine intermediate wraps:
126+
//nolint:forbidigo with a reason.
97127
# ── http: shortcuts must not construct raw HTTP requests ──
98128
# Bans request / client construction; constants (http.MethodPost,
99129
# http.StatusOK) and pure helpers (http.StatusText, http.Header) are

errs/subtypes.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ const (
1212

1313
// CategoryValidation subtypes
1414
const (
15-
SubtypeInvalidArgument Subtype = "invalid_argument" // user-supplied flag / arg failed validation (gRPC INVALID_ARGUMENT alignment)
15+
SubtypeInvalidArgument Subtype = "invalid_argument" // user-supplied flag / arg failed validation (gRPC INVALID_ARGUMENT alignment)
16+
SubtypeFailedPrecondition Subtype = "failed_precondition" // request is valid but the system/resource state is not in the state required to execute; caller must change state (not retry) — e.g. ambiguous remote mapping (gRPC FAILED_PRECONDITION alignment)
1617
)
1718

1819
// CategoryAuthentication subtypes

errs/types.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,22 @@ type TypedError interface {
6161
// it is intentionally not serialized.
6262
type ValidationError struct {
6363
Problem
64-
Param string `json:"param,omitempty"`
65-
Cause error `json:"-"`
64+
Param string `json:"param,omitempty"`
65+
Params []InvalidParam `json:"params,omitempty"`
66+
Cause error `json:"-"`
67+
}
68+
69+
// InvalidParam is one structured validation diagnostic: the parameter that
70+
// failed (Name) and why (Reason). It mirrors an RFC 7807 "invalid-params"
71+
// item (RFC 7807 §3.1 extension members).
72+
//
73+
// The wire key on ValidationError is "params" rather than "invalid_params"
74+
// because the enclosing envelope already carries type:"validation", so the
75+
// "invalid" qualifier would be redundant on the wire. The Go type keeps the
76+
// InvalidParam prefix because, at package level, the name must self-describe.
77+
type InvalidParam struct {
78+
Name string `json:"name"`
79+
Reason string `json:"reason"`
6680
}
6781

6882
// Unwrap exposes the wrapped cause so errors.Unwrap / errors.Is can traverse
@@ -122,6 +136,11 @@ func (e *ValidationError) WithParam(param string) *ValidationError {
122136
return e
123137
}
124138

139+
func (e *ValidationError) WithParams(params ...InvalidParam) *ValidationError {
140+
e.Params = append(e.Params, params...)
141+
return e
142+
}
143+
125144
func (e *ValidationError) WithCause(cause error) *ValidationError {
126145
e.Cause = cause
127146
return e

errs/types_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,71 @@ func TestTypedError_UnwrapSymmetry(t *testing.T) {
558558
})
559559
}
560560

561+
// TestValidationError_WithParams covers the structured-validation extension:
562+
// WithParams appends InvalidParam items, the scalar Param setter is unaffected,
563+
// and the wire shape nests {name, reason} under "params" (omitted when empty).
564+
func TestValidationError_WithParams(t *testing.T) {
565+
t.Run("appends and exposes fields", func(t *testing.T) {
566+
e := errs.NewValidationError(errs.SubtypeInvalidArgument, "duplicate rel_path").
567+
WithParams(errs.InvalidParam{Name: "a.md", Reason: "duplicate"})
568+
if len(e.Params) != 1 {
569+
t.Fatalf("len(Params) = %d, want 1", len(e.Params))
570+
}
571+
if e.Params[0].Name != "a.md" {
572+
t.Errorf("Params[0].Name = %q, want %q", e.Params[0].Name, "a.md")
573+
}
574+
if e.Params[0].Reason != "duplicate" {
575+
t.Errorf("Params[0].Reason = %q, want %q", e.Params[0].Reason, "duplicate")
576+
}
577+
})
578+
579+
t.Run("appends across multiple calls and returns receiver", func(t *testing.T) {
580+
e := errs.NewValidationError(errs.SubtypeInvalidArgument, "x")
581+
returned := e.WithParams(errs.InvalidParam{Name: "a.md", Reason: "dup"})
582+
if returned != e {
583+
t.Errorf("WithParams returned different pointer; want same as receiver")
584+
}
585+
e.WithParams(
586+
errs.InvalidParam{Name: "b.md", Reason: "dup"},
587+
errs.InvalidParam{Name: "c.md", Reason: "dup"},
588+
)
589+
if len(e.Params) != 3 {
590+
t.Fatalf("len(Params) = %d after two calls, want 3", len(e.Params))
591+
}
592+
})
593+
594+
t.Run("wire shape nests name and reason under params", func(t *testing.T) {
595+
e := errs.NewValidationError(errs.SubtypeInvalidArgument, "duplicate rel_path").
596+
WithParam("--rel-path").
597+
WithParams(errs.InvalidParam{Name: "a.md", Reason: "duplicate"})
598+
b, err := json.Marshal(e)
599+
if err != nil {
600+
t.Fatalf("marshal failed: %v", err)
601+
}
602+
got := string(b)
603+
for _, want := range []string{
604+
`"type":"validation"`,
605+
`"param":"--rel-path"`,
606+
`"params":[{"name":"a.md","reason":"duplicate"}]`,
607+
} {
608+
if !strings.Contains(got, want) {
609+
t.Errorf("missing %q in %s", want, got)
610+
}
611+
}
612+
})
613+
614+
t.Run("empty Params omitted from wire", func(t *testing.T) {
615+
e := errs.NewValidationError(errs.SubtypeInvalidArgument, "x")
616+
b, err := json.Marshal(e)
617+
if err != nil {
618+
t.Fatalf("marshal failed: %v", err)
619+
}
620+
if strings.Contains(string(b), `"params"`) {
621+
t.Errorf("empty Params should be omitted from wire; got %s", b)
622+
}
623+
})
624+
}
625+
561626
func TestBuilderSetter_DefensiveCopy(t *testing.T) {
562627
t.Run("WithMissingScopes clones input", func(t *testing.T) {
563628
scopes := []string{"docx:document", "im:message:send"}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
2+
// SPDX-License-Identifier: MIT
3+
4+
package errclass
5+
6+
import "github.com/larksuite/cli/errs"
7+
8+
// driveCodeMeta holds drive/docs-service Lark code → CodeMeta mappings.
9+
// Only codes whose meaning is verifiable from repo evidence are registered;
10+
// ambiguous codes fall back to CategoryAPI via BuildAPIError.
11+
// BuildAPIError consumes this map via mergeCodeMeta + LookupCodeMeta.
12+
var driveCodeMeta = map[int]CodeMeta{
13+
1061044: {Category: errs.CategoryAPI, Subtype: errs.SubtypeNotFound}, // parent folder does not exist (upload)
14+
1069302: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // comment endpoint "Invalid or missing parameters"
15+
}
16+
17+
func init() { mergeCodeMeta(driveCodeMeta, "drive") }
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
2+
// SPDX-License-Identifier: MIT
3+
4+
package errclass
5+
6+
import (
7+
"fmt"
8+
"testing"
9+
10+
"github.com/larksuite/cli/errs"
11+
)
12+
13+
// TestLookupCodeMeta_DriveCodes pins each drive-service code registered via the
14+
// codemeta_drive.go init() merge to its expected Category/Subtype/Retryable.
15+
// Each case traces to repo evidence (see codemeta_drive.go comments).
16+
func TestLookupCodeMeta_DriveCodes(t *testing.T) {
17+
cases := []struct {
18+
code int
19+
wantCat errs.Category
20+
wantSubtype errs.Subtype
21+
wantRetry bool
22+
}{
23+
// 1061044: upload with a nonexistent parent folder token. The drive E2E
24+
// (tests_e2e/drive/2026_06_01_errs_migrate_drive_test.go) drives this
25+
// producer via a nonexistent parent folder → referenced resource missing.
26+
{1061044, errs.CategoryAPI, errs.SubtypeNotFound, false},
27+
// 1069302: comment endpoint's opaque "Invalid or missing parameters"
28+
// (shortcuts/drive/drive_add_comment.go) → API-side parameter rejection.
29+
{1069302, errs.CategoryAPI, errs.SubtypeInvalidParameters, false},
30+
}
31+
for _, tc := range cases {
32+
t.Run(fmt.Sprintf("%d", tc.code), func(t *testing.T) {
33+
meta, ok := LookupCodeMeta(tc.code)
34+
if !ok {
35+
t.Fatalf("code %d not registered in codeMeta", tc.code)
36+
}
37+
if meta.Category != tc.wantCat || meta.Subtype != tc.wantSubtype || meta.Retryable != tc.wantRetry {
38+
t.Errorf("code %d: got %+v, want Category=%v Subtype=%v Retryable=%v",
39+
tc.code, meta, tc.wantCat, tc.wantSubtype, tc.wantRetry)
40+
}
41+
})
42+
}
43+
}
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
2+
// SPDX-License-Identifier: MIT
3+
4+
package errscontract
5+
6+
import (
7+
"go/ast"
8+
"go/parser"
9+
"go/token"
10+
"strings"
11+
)
12+
13+
// migratedEnvelopePaths lists the source-tree prefixes that have been migrated
14+
// to the typed errs.* taxonomy. On these paths, constructing a legacy
15+
// output.ExitError / output.ErrDetail envelope literal directly is forbidden —
16+
// call sites must return a typed errs.* error instead. Future domains opt in by
17+
// appending their path prefix here.
18+
var migratedEnvelopePaths = []string{
19+
"shortcuts/drive/",
20+
}
21+
22+
// legacyOutputImportPath is the import path of the package that declares the
23+
// legacy ExitError / ErrDetail envelope types. The rule resolves whatever local
24+
// name (default or alias) this path is bound to in each file, so an aliased
25+
// import cannot bypass the check.
26+
const legacyOutputImportPath = "github.com/larksuite/cli/internal/output"
27+
28+
// CheckNoLegacyEnvelopeLiteral flags direct construction of legacy
29+
// output.ExitError / output.ErrDetail composite literals on migrated paths.
30+
// forbidigo can ban identifiers but not composite literals, so this AST rule
31+
// covers the gap left after a path is migrated to typed errs.* errors.
32+
//
33+
// Path-scoped to migratedEnvelopePaths (mirrors how CheckProblemEmbed restricts
34+
// by path); skips _test.go fixtures. output.ErrBare(...) is a CallExpr, not a
35+
// CompositeLit, so the predicate exit-signal helper is naturally not flagged.
36+
func CheckNoLegacyEnvelopeLiteral(path, src string) []Violation {
37+
if !isMigratedEnvelopePath(path) || strings.HasSuffix(path, "_test.go") {
38+
return nil
39+
}
40+
fset := token.NewFileSet()
41+
file, err := parser.ParseFile(fset, path, src, parser.ParseComments)
42+
if err != nil {
43+
return nil
44+
}
45+
// Resolve the local name(s) bound to the legacy output import path. A file
46+
// may bind it as the default `output`, an alias (`legacy "...output"`), or a
47+
// dot-import (qualifier becomes ""), in which case ExitError/ErrDetail appear
48+
// as bare unqualified idents.
49+
localNames, dotImported := resolveLegacyOutputNames(file)
50+
var out []Violation
51+
ast.Inspect(file, func(n ast.Node) bool {
52+
lit, ok := n.(*ast.CompositeLit)
53+
if !ok {
54+
return true
55+
}
56+
if name, ok := legacyEnvelopeTypeName(lit.Type, localNames, dotImported); ok {
57+
out = append(out, Violation{
58+
Rule: "no_legacy_envelope_literal",
59+
Action: ActionReject,
60+
File: path,
61+
Line: fset.Position(lit.Pos()).Line,
62+
Message: "direct construction of legacy output." + name + " is forbidden on migrated paths; return a typed errs.* error (output.ErrBare remains allowed for predicate exit signals)",
63+
Suggestion: "replace the &output." + name + "{...} literal with a typed errs.* constructor " +
64+
"(e.g. errs.NewValidationError / errs.NewAPIError / errs.NewNetworkError)",
65+
})
66+
}
67+
return true
68+
})
69+
return out
70+
}
71+
72+
// isMigratedEnvelopePath reports whether path falls under any migrated path
73+
// prefix in migratedEnvelopePaths.
74+
func isMigratedEnvelopePath(path string) bool {
75+
p := strings.ReplaceAll(path, "\\", "/")
76+
for _, prefix := range migratedEnvelopePaths {
77+
if strings.HasPrefix(p, prefix) || strings.Contains(p, "/"+prefix) {
78+
return true
79+
}
80+
}
81+
return false
82+
}
83+
84+
// resolveLegacyOutputNames walks the file's import declarations and returns the
85+
// set of local names bound to legacyOutputImportPath, plus whether the path was
86+
// dot-imported. Default imports bind the package's own name ("output"); aliased
87+
// imports bind the alias; dot-imports bind names into the file scope.
88+
func resolveLegacyOutputNames(file *ast.File) (map[string]struct{}, bool) {
89+
names := make(map[string]struct{})
90+
dotImported := false
91+
for _, imp := range file.Imports {
92+
if imp.Path == nil {
93+
continue
94+
}
95+
p := strings.Trim(imp.Path.Value, "`\"")
96+
if p != legacyOutputImportPath {
97+
continue
98+
}
99+
switch {
100+
case imp.Name == nil:
101+
// Default import: local name is the package name "output".
102+
names["output"] = struct{}{}
103+
case imp.Name.Name == ".":
104+
dotImported = true
105+
case imp.Name.Name == "_":
106+
// Blank import cannot reference the types; ignore.
107+
default:
108+
names[imp.Name.Name] = struct{}{}
109+
}
110+
}
111+
return names, dotImported
112+
}
113+
114+
// legacyEnvelopeTypeName reports whether a composite-literal Type names the
115+
// legacy ExitError / ErrDetail envelope and returns the bare type name. It
116+
// matches a qualified selector (pkg.ExitError) when pkg is one of the resolved
117+
// local names for the legacy output import, and — when the package was
118+
// dot-imported — also matches a bare unqualified ExitError / ErrDetail ident.
119+
func legacyEnvelopeTypeName(expr ast.Expr, localNames map[string]struct{}, dotImported bool) (string, bool) {
120+
if sel, ok := expr.(*ast.SelectorExpr); ok {
121+
x, ok := sel.X.(*ast.Ident)
122+
if !ok || sel.Sel == nil {
123+
return "", false
124+
}
125+
if _, bound := localNames[x.Name]; !bound {
126+
return "", false
127+
}
128+
return matchLegacyEnvelopeName(sel.Sel.Name)
129+
}
130+
if dotImported {
131+
if ident, ok := expr.(*ast.Ident); ok {
132+
return matchLegacyEnvelopeName(ident.Name)
133+
}
134+
}
135+
return "", false
136+
}
137+
138+
// matchLegacyEnvelopeName returns the name when it is one of the legacy
139+
// envelope type names.
140+
func matchLegacyEnvelopeName(name string) (string, bool) {
141+
switch name {
142+
case "ExitError", "ErrDetail":
143+
return name, true
144+
}
145+
return "", false
146+
}

0 commit comments

Comments
 (0)