Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,20 @@ linters:
- forbidigo
# errs-typed-only enforced on paths already migrated to errs.NewXxxError.
# Add a path when its migration is complete.
- 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/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/)
- 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/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/)
text: errs-typed-only
linters:
- forbidigo
# errs-no-bare-wrap enforced on paths fully migrated to typed final
# errors. Scoped separately from errs-typed-only because cmd/auth/,
# cmd/config/ still have residual fmt.Errorf and must not be caught.
- path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/common/mcp_client\.go)
- path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/|shortcuts/common/mcp_client\.go)
text: errs-no-bare-wrap
linters:
- forbidigo
# errs-no-legacy-helper enforced on domains whose shared validation/save
# helpers have migrated to typed final errors.
- path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/)
- path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/)
text: errs-no-legacy-helper
linters:
- forbidigo
Expand Down
4 changes: 2 additions & 2 deletions cmd/root_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,9 +377,9 @@ func TestIntegration_Shortcut_BusinessError_OutputsEnvelope(t *testing.T) {
OK: false,
Identity: "bot",
Error: &output.ErrDetail{
Type: "api_error",
Type: "api",
Code: 230002,
Message: "HTTP 400: Bot/User can NOT be out of the chat.",
Message: "Bot/User can NOT be out of the chat.",
},
})
}
Expand Down
1 change: 1 addition & 0 deletions lint/errscontract/rule_no_legacy_envelope_literal.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ var migratedEnvelopePaths = []string{
"shortcuts/okr/",
"shortcuts/task/",
"shortcuts/whiteboard/",
"shortcuts/im/",
}

// legacyOutputImportPath is the import path of the package that declares the
Expand Down
10 changes: 5 additions & 5 deletions lint/errscontract/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ func boom() error {
return &output.ExitError{Code: 1}
}
`
v := CheckNoLegacyEnvelopeLiteral("shortcuts/im/foo.go", src)
v := CheckNoLegacyEnvelopeLiteral("shortcuts/contact/foo.go", src)
if len(v) != 0 {
t.Errorf("non-migrated path should pass, got: %+v", v)
}
Expand Down Expand Up @@ -900,14 +900,14 @@ func boom(runtime *common.RuntimeContext) error {
}

func TestCheckNoLegacyRuntimeAPICall_IgnoresNonMigratedPath(t *testing.T) {
src := `package im
src := `package contact

func boom(runtime *common.RuntimeContext) error {
_, err := runtime.CallAPI("POST", "/x", nil, nil)
return err
}
`
v := CheckNoLegacyRuntimeAPICall("shortcuts/im/im_send.go", src)
v := CheckNoLegacyRuntimeAPICall("shortcuts/contact/contact_get.go", src)
if len(v) != 0 {
t.Errorf("non-migrated path must not fire, got: %+v", v)
}
Expand Down Expand Up @@ -998,15 +998,15 @@ func boom() {
}

func TestCheckNoLegacyCommonHelperCall_AllowsNonMigratedPath(t *testing.T) {
src := `package im
src := `package contact

import "github.com/larksuite/cli/shortcuts/common"

func boom() {
common.FlagErrorf("legacy allowed until domain migrates")
}
`
v := CheckNoLegacyCommonHelperCall("shortcuts/im/im_send.go", src)
v := CheckNoLegacyCommonHelperCall("shortcuts/contact/contact_get.go", src)
if len(v) != 0 {
t.Errorf("non-migrated path must pass, got: %+v", v)
}
Expand Down
56 changes: 56 additions & 0 deletions shortcuts/common/call_api_typed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/spf13/cobra"

"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/internal/client"
"github.com/larksuite/cli/internal/cmdutil"
"github.com/larksuite/cli/internal/core"
"github.com/larksuite/cli/internal/httpmock"
Expand Down Expand Up @@ -198,3 +199,58 @@ func TestCallAPITyped_NonObjectJSON(t *testing.T) {
t.Errorf("subtype = %q, want %q", intErr.Subtype, errs.SubtypeInvalidResponse)
}
}

// TestDoAPIJSONTyped_Success returns the data object on code 0, confirming the
// typed DoAPIJSON replacement preserves the success contract of DoAPIJSON.
func TestDoAPIJSONTyped_Success(t *testing.T) {
rt, reg := newCallAPITypedRuntime(t)
reg.Register(&httpmock.Stub{
Method: "GET",
URL: "/open-apis/x/z",
Body: map[string]interface{}{"code": float64(0), "data": map[string]interface{}{"id": "z1"}},
})

data, err := rt.DoAPIJSONTyped("GET", "/open-apis/x/z", nil, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if data["id"] != "z1" {
t.Errorf("data[id] = %v, want z1", data["id"])
}
}

func TestDoAPIJSONTyped_RawClientErrorBecomesTypedInternal(t *testing.T) {
rt := TestNewRuntimeContextForAPI(context.Background(), &cobra.Command{Use: "+x"}, &core.CliConfig{}, nil, core.AsUser)
rt.apiClientFunc = func() (*client.APIClient, error) {
return nil, errors.New("raw client construction error")
}

_, err := rt.DoAPIJSONTyped("GET", "/open-apis/x/z", nil, nil)
var internalErr *errs.InternalError
if !errors.As(err, &internalErr) {
t.Fatalf("expected raw client errors to be lifted to typed internal errors, got %T: %v", err, err)
}
if internalErr.Subtype != errs.SubtypeUnknown {
t.Errorf("subtype = %q, want %q", internalErr.Subtype, errs.SubtypeUnknown)
}
}

// TestDoAPIJSONTyped_NonZeroCode classifies a non-zero API code into a typed
// errs.* error (carrying log_id), never a legacy output.ExitError envelope.
func TestDoAPIJSONTyped_NonZeroCode(t *testing.T) {
rt, reg := newCallAPITypedRuntime(t)
reg.Register(&httpmock.Stub{
Method: "POST",
URL: "/open-apis/x/z",
Body: map[string]interface{}{"code": float64(1061044), "msg": "boom", "log_id": "lz"},
})

_, err := rt.DoAPIJSONTyped("POST", "/open-apis/x/z", nil, map[string]any{})
p, ok := errs.ProblemOf(err)
if !ok {
t.Fatalf("expected a typed errs.* error, got %T: %v", err, err)
}
if p.LogID != "lz" {
t.Errorf("LogID = %q, want lz", p.LogID)
}
}
25 changes: 25 additions & 0 deletions shortcuts/common/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,28 @@ func (ctx *RuntimeContext) DoAPIJSONWithLogID(method, apiPath string, query lark
return ctx.doAPIJSON(method, apiPath, query, body, true)
}

// DoAPIJSONTyped is the typed-only replacement for DoAPIJSON: it issues the same
// larkcore.ApiReq request (identical method / path / query / body model) but
// classifies failures into typed errs.* errors via ClassifyAPIResponse instead
// of emitting a legacy output.ExitError "api_error" envelope. A transport / auth
// error from the client boundary is already typed and passes through unchanged;
// a non-zero API code is classified with subtype / code / log_id.
func (ctx *RuntimeContext) DoAPIJSONTyped(method, apiPath string, query larkcore.QueryParams, body any) (map[string]any, error) {
req := &larkcore.ApiReq{
HttpMethod: method,
ApiPath: apiPath,
QueryParams: query,
}
if body != nil {
req.Body = body
}
resp, err := ctx.DoAPI(req)
if err != nil {
return nil, typedOrInternal(err)
}
return ctx.ClassifyAPIResponse(resp)
}

func (ctx *RuntimeContext) doAPIJSON(method, apiPath string, query larkcore.QueryParams, body any, includeLogID bool) (map[string]any, error) {
req := &larkcore.ApiReq{
HttpMethod: method,
Expand Down Expand Up @@ -682,6 +704,9 @@ func WrapSaveErrorTyped(err error) error {
if err == nil {
return nil
}
if _, ok := errs.ProblemOf(err); ok {
return err
}
var me *fileio.MkdirError
switch {
case errors.Is(err, fileio.ErrPathValidation):
Expand Down
12 changes: 0 additions & 12 deletions shortcuts/common/validate_ids.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,6 @@ import (
"github.com/larksuite/cli/internal/output"
)

// ValidateChatID checks if a chat ID has valid format (oc_ prefix).
// Also extracts token from URL if provided.
//
// Deprecated: use ValidateChatIDTyped for typed error envelopes.
func ValidateChatID(input string) (string, error) {
chatID, msg := normalizeChatID(input)
if msg != "" {
return "", output.ErrValidation("%s", msg)
}
return chatID, nil
}

// ValidateChatIDTyped checks if a chat ID has valid format (oc_ prefix).
// Also extracts token from URL if provided. param names the flag being
// validated (e.g. "--chat-ids") and is recorded on the typed error.
Expand Down
15 changes: 15 additions & 0 deletions shortcuts/common/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,21 @@ func TestWrapSaveErrorTyped_ClassifiesPathAndFileIO(t *testing.T) {
}
}

func TestWrapSaveErrorTyped_PreservesTypedWriteCause(t *testing.T) {
typed := errs.NewNetworkError(errs.SubtypeNetworkServer, "HTTP 500: chunk failed").
WithCode(500)
err := WrapSaveErrorTyped(&fileio.WriteError{Err: typed})

p, ok := errs.ProblemOf(err)
if !ok {
t.Fatalf("expected typed problem, got %T: %v", err, err)
}
if p.Category != errs.CategoryNetwork || p.Subtype != errs.SubtypeNetworkServer || p.Code != 500 {
t.Fatalf("problem = category %q subtype %q code %d, want network/%s/500",
p.Category, p.Subtype, p.Code, errs.SubtypeNetworkServer)
}
}

func TestAtLeastOne(t *testing.T) {
tests := []struct {
name string
Expand Down
4 changes: 2 additions & 2 deletions shortcuts/im/convert_lib/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func batchResolveByBasicContact(runtime *common.RuntimeContext, missingIDs []str
}
batch := missingIDs[i:end]

data, err := runtime.DoAPIJSON(http.MethodPost,
data, err := runtime.DoAPIJSONTyped(http.MethodPost,
"/open-apis/contact/v3/users/basic_batch",
larkcore.QueryParams{"user_id_type": []string{"open_id"}},
map[string]interface{}{"user_ids": batch},
Expand Down Expand Up @@ -198,7 +198,7 @@ func batchResolveUsers(runtime *common.RuntimeContext, missingIDs []string, name
}
apiURL := "/open-apis/contact/v3/users/batch?" + strings.Join(parts, "&")

data, err := runtime.DoAPIJSON(http.MethodGet, apiURL, nil, nil)
data, err := runtime.DoAPIJSONTyped(http.MethodGet, apiURL, nil, nil)
if err != nil {
break
}
Expand Down
6 changes: 3 additions & 3 deletions shortcuts/im/convert_lib/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,20 +200,20 @@ func batchResolveMergeForwardSenders(runtime *common.RuntimeContext, prefetch ma
// container via a single API call. Returns a flat list of raw message items
// with upper_message_id for tree reconstruction.
//
// Uses DoAPIJSON so the response envelope's code/msg are checked and surfaced
// Uses DoAPIJSONTyped so the response envelope's code/msg are checked and surfaced
// — earlier this used the low-level DoAPI and reported every non-zero code
// as a generic "empty data" error, hiding the real failure (e.g. a server
// "code: 2200 Internal Error" with its log_id would show up as just "empty
// data" in the output).
func fetchMergeForwardSubMessages(messageID string, runtime *common.RuntimeContext) ([]map[string]interface{}, error) {
data, err := runtime.DoAPIJSON(http.MethodGet, mergeForwardMessagesPath(messageID), larkcore.QueryParams{
data, err := runtime.DoAPIJSONTyped(http.MethodGet, mergeForwardMessagesPath(messageID), larkcore.QueryParams{
"user_id_type": []string{"open_id"},
"card_msg_content_type": []string{"raw_card_content"},
}, nil)
if err != nil {
return nil, err
}
// DoAPIJSON returns the envelope's `data` field; when the server's JSON
// DoAPIJSONTyped returns the envelope's `data` field; when the server's JSON
// has `code: 0` but omits `data` entirely, that field comes back as nil.
// Reading from a nil map in Go is safe (returns the zero value, never
// panics), but guarding explicitly makes the "successful empty
Expand Down
2 changes: 1 addition & 1 deletion shortcuts/im/convert_lib/reactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func fetchReactionsBatch(runtime *common.RuntimeContext, batchIDs []string, idIn
queries = append(queries, map[string]interface{}{"message_id": id})
}

data, err := runtime.DoAPIJSON(http.MethodPost,
data, err := runtime.DoAPIJSONTyped(http.MethodPost,
"/open-apis/im/v1/messages/reactions/batch_query",
nil,
map[string]interface{}{"queries": queries},
Expand Down
4 changes: 2 additions & 2 deletions shortcuts/im/convert_lib/thread.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,15 +243,15 @@ func ExpandThreadReplies(runtime *common.RuntimeContext, messages []map[string]i
// Returns the raw message items, whether more replies exist beyond the limit,
// and a non-nil error when the API call fails.
func fetchThreadReplies(runtime *common.RuntimeContext, threadID string, limit int) ([]map[string]interface{}, bool, error) {
data, err := runtime.DoAPIJSON(http.MethodGet, "/open-apis/im/v1/messages", larkcore.QueryParams{
data, err := runtime.DoAPIJSONTyped(http.MethodGet, "/open-apis/im/v1/messages", larkcore.QueryParams{
"container_id_type": []string{"thread"},
"container_id": []string{threadID},
"sort_type": []string{"ByCreateTimeAsc"},
"page_size": []string{fmt.Sprint(limit)},
"card_msg_content_type": []string{"raw_card_content"},
}, nil)
if err != nil {
return nil, false, fmt.Errorf("fetch thread replies for %s: %w", threadID, err)
return nil, false, fmt.Errorf("fetch thread replies for %s: %w", threadID, err) //nolint:forbidigo // best-effort internal thread fetch; never surfaced as a final shortcut error (ExpandThreadReplies is void)
}
hasMore, _ := data["has_more"].(bool)
rawItems, _ := data["items"].([]interface{})
Expand Down
Loading
Loading