diff --git a/.golangci.yml b/.golangci.yml index f9a51eae8..22e00f1d5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -65,20 +65,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/calendar/helpers\.go|shortcuts/drive/) + - 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/|shortcuts/minutes/|shortcuts/vc/) 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/drive/|shortcuts/calendar/helpers\.go|shortcuts/common/mcp_client\.go) + - path-except: (shortcuts/drive/|shortcuts/minutes/|shortcuts/vc/|shortcuts/calendar/helpers\.go|shortcuts/common/mcp_client\.go) text: errs-no-bare-wrap linters: - forbidigo # errs-no-legacy-helper is drive-only: the shared helpers it bans are # still used by other domains until their later migration phase. - - path-except: (shortcuts/drive/) + - path-except: (shortcuts/drive/|shortcuts/minutes/|shortcuts/vc/) text: errs-no-legacy-helper linters: - forbidigo diff --git a/internal/errclass/codemeta_minutes.go b/internal/errclass/codemeta_minutes.go new file mode 100644 index 000000000..06686e995 --- /dev/null +++ b/internal/errclass/codemeta_minutes.go @@ -0,0 +1,18 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package errclass + +import "github.com/larksuite/cli/errs" + +// minutesCodeMeta holds minutes-service Lark code → CodeMeta mappings. +// Only codes whose meaning is verifiable from repo evidence are registered; +// ambiguous codes fall back to CategoryAPI via BuildAPIError. Command-specific +// messages and hints are layered on top via per-command enrichment. +// BuildAPIError consumes this map via mergeCodeMeta + LookupCodeMeta. +var minutesCodeMeta = map[int]CodeMeta{ + 2091001: {Category: errs.CategoryAPI, Subtype: errs.SubtypeNotFound}, // speaker not found in transcript + 2091005: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypePermissionDenied}, // caller lacks edit/read permission for the minute +} + +func init() { mergeCodeMeta(minutesCodeMeta, "minutes") } diff --git a/internal/errclass/codemeta_vc.go b/internal/errclass/codemeta_vc.go new file mode 100644 index 000000000..f9ab72c1d --- /dev/null +++ b/internal/errclass/codemeta_vc.go @@ -0,0 +1,19 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package errclass + +import "github.com/larksuite/cli/errs" + +// vcCodeMeta holds vc-service Lark code → CodeMeta mappings. +// Only codes whose meaning is verifiable from repo evidence are registered; +// ambiguous codes (e.g. 124002 "recording still generating", which has no +// precise taxonomy fit) fall back to CategoryAPI via BuildAPIError and rely on +// per-command enrichment for a retry hint. +// BuildAPIError consumes this map via mergeCodeMeta + LookupCodeMeta. +var vcCodeMeta = map[int]CodeMeta{ + 121004: {Category: errs.CategoryAPI, Subtype: errs.SubtypeNotFound}, // meeting has no minute file + 121005: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypePermissionDenied}, // caller is not a participant / lacks view permission +} + +func init() { mergeCodeMeta(vcCodeMeta, "vc") } diff --git a/lint/errscontract/rule_no_legacy_envelope_literal.go b/lint/errscontract/rule_no_legacy_envelope_literal.go index b995b27f2..d4cde1a2e 100644 --- a/lint/errscontract/rule_no_legacy_envelope_literal.go +++ b/lint/errscontract/rule_no_legacy_envelope_literal.go @@ -17,6 +17,8 @@ import ( // appending their path prefix here. var migratedEnvelopePaths = []string{ "shortcuts/drive/", + "shortcuts/minutes/", + "shortcuts/vc/", } // legacyOutputImportPath is the import path of the package that declares the diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index ac9bcd0c6..31f06fa30 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -17,6 +17,7 @@ import ( "strings" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" @@ -54,21 +55,21 @@ var MinutesDownload = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { tokens := common.SplitCSV(runtime.Str("minute-tokens")) if len(tokens) == 0 { - return output.ErrValidation("--minute-tokens is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--minute-tokens is required").WithParam("--minute-tokens") } if len(tokens) > maxBatchSize { - return output.ErrValidation("--minute-tokens: too many tokens (%d), maximum is %d", len(tokens), maxBatchSize) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--minute-tokens: too many tokens (%d), maximum is %d", len(tokens), maxBatchSize).WithParam("--minute-tokens") } for _, token := range tokens { if !validMinuteToken.MatchString(token) { - return output.ErrValidation("invalid minute token %q: must contain only lowercase alphanumeric characters (e.g. obcnq3b9jl72l83w4f149w9c)", token) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid minute token %q: must contain only lowercase alphanumeric characters (e.g. obcnq3b9jl72l83w4f149w9c)", token).WithParam("--minute-tokens") } } // Cheap checks first, then path-safety resolution. out := runtime.Str("output") outDir := runtime.Str("output-dir") if out != "" && outDir != "" { - return output.ErrValidation("--output and --output-dir cannot both be set") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--output and --output-dir cannot both be set").WithParam("--output") } if out != "" { if err := common.ValidateSafePath(runtime.FileIO(), out); err != nil { @@ -112,7 +113,7 @@ var MinutesDownload = common.Shortcut{ explicitOutputPath = "" case statErr == nil && !fi.IsDir(): if !single { - return output.ErrValidation("--output %q is a file; batch mode expects a directory (use --output-dir)", explicitOutputPath) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--output %q is a file; batch mode expects a directory (use --output-dir)", explicitOutputPath).WithParam("--output") } case errors.Is(statErr, fs.ErrNotExist): if !single { @@ -120,7 +121,7 @@ var MinutesDownload = common.Shortcut{ explicitOutputPath = "" } default: - return output.Errorf(output.ExitAPI, "io_error", "cannot access --output %q: %s", explicitOutputPath, statErr) + return errs.NewInternalError(errs.SubtypeFileIO, "cannot access --output %q: %s", explicitOutputPath, statErr).WithCause(statErr) } } @@ -137,6 +138,7 @@ var MinutesDownload = common.Shortcut{ SizeBytes int64 `json:"size_bytes,omitempty"` DownloadURL string `json:"download_url,omitempty"` Error string `json:"error,omitempty"` + err error // raw typed error for single-mode passthrough } results := make([]result, len(tokens)) @@ -151,18 +153,18 @@ var MinutesDownload = common.Shortcut{ // download URLs originate from the trusted Lark API, not user input. baseClient, err := runtime.Factory.HttpClient() if err != nil { - return output.ErrNetwork("failed to get HTTP client: %s", err) + return errs.NewNetworkError(errs.SubtypeNetworkTransport, "failed to get HTTP client: %s", err).WithCause(err) } clonedClient := *baseClient clonedClient.Timeout = disableClientTimeout clonedClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { if len(via) >= maxDownloadRedirects { - return fmt.Errorf("too many redirects") + return fmt.Errorf("too many redirects") //nolint:forbidigo // returned to net/http CheckRedirect, not a CLI terminal error } if len(via) > 0 { prev := via[len(via)-1] if strings.EqualFold(prev.URL.Scheme, "https") && strings.EqualFold(req.URL.Scheme, "http") { - return fmt.Errorf("redirect from https to http is not allowed") + return fmt.Errorf("redirect from https to http is not allowed") //nolint:forbidigo // returned to net/http CheckRedirect, not a CLI terminal error } } return validate.ValidateDownloadSourceURL(req.Context(), req.URL.String()) @@ -193,7 +195,7 @@ var MinutesDownload = common.Shortcut{ downloadURL, err := fetchDownloadURL(ctx, runtime, token) if err != nil { - results[i] = result{MinuteToken: token, Error: err.Error()} + results[i] = result{MinuteToken: token, Error: err.Error(), err: err} continue } @@ -220,7 +222,7 @@ var MinutesDownload = common.Shortcut{ dl, err := downloadMediaFile(ctx, dlClient, downloadURL, token, opts) if err != nil { - results[i] = result{MinuteToken: token, Error: err.Error()} + results[i] = result{MinuteToken: token, Error: err.Error(), err: err} continue } results[i] = result{ @@ -235,7 +237,10 @@ var MinutesDownload = common.Shortcut{ if single { r := results[0] if r.Error != "" { - return output.ErrAPI(0, r.Error, nil) + if r.err != nil { + return r.err // typed error from fetchDownloadURL/downloadMediaFile, exit code preserved + } + return runtime.OutPartialFailure(map[string]interface{}{"downloads": results}, &output.Meta{Count: len(results)}) } if urlOnly { runtime.Out(map[string]interface{}{ @@ -262,17 +267,19 @@ var MinutesDownload = common.Shortcut{ } fmt.Fprintf(errOut, "[minutes +download] done: %d total, %d succeeded, %d failed\n", len(results), successCount, len(results)-successCount) - runtime.OutFormat(map[string]interface{}{"downloads": results}, &output.Meta{Count: len(results)}, nil) + outData := map[string]interface{}{"downloads": results} + meta := &output.Meta{Count: len(results)} if successCount == 0 && len(results) > 0 { - return output.ErrAPI(0, fmt.Sprintf("all %d downloads failed", len(results)), nil) + return runtime.OutPartialFailure(outData, meta) } + runtime.OutFormat(outData, meta, nil) return nil }, } // fetchDownloadURL retrieves the pre-signed download URL for a minute token. func fetchDownloadURL(ctx context.Context, runtime *common.RuntimeContext, minuteToken string) (string, error) { - data, err := runtime.DoAPIJSON(http.MethodGet, + data, err := runtime.CallAPITyped(http.MethodGet, fmt.Sprintf("/open-apis/minutes/v1/minutes/%s/media", validate.EncodePathSegment(minuteToken)), nil, nil) if err != nil { @@ -280,7 +287,7 @@ func fetchDownloadURL(ctx context.Context, runtime *common.RuntimeContext, minut } downloadURL := common.GetString(data, "download_url") if downloadURL == "" { - return "", output.Errorf(output.ExitAPI, "api_error", "API returned empty download_url for %s", minuteToken) + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "API returned empty download_url for %s", minuteToken) } return downloadURL, nil } @@ -302,26 +309,26 @@ type downloadOpts struct { // Filename resolution: opts.outputPath > Content-Disposition filename > Content-Type ext > .media. func downloadMediaFile(ctx context.Context, client *http.Client, downloadURL, minuteToken string, opts downloadOpts) (*downloadResult, error) { if err := validate.ValidateDownloadSourceURL(ctx, downloadURL); err != nil { - return nil, output.ErrValidation("blocked download URL: %s", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "blocked download URL: %s", err).WithCause(err) } req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) if err != nil { - return nil, output.ErrNetwork("invalid download URL: %s", err) + return nil, errs.NewNetworkError(errs.SubtypeNetworkTransport, "invalid download URL: %s", err).WithCause(err) } resp, err := client.Do(req) if err != nil { - return nil, output.ErrNetwork("download failed: %s", err) + return nil, errs.NewNetworkError(errs.SubtypeNetworkTransport, "download failed: %s", err).WithCause(err) } defer resp.Body.Close() if resp.StatusCode < 200 || resp.StatusCode >= 300 { body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) if len(body) > 0 { - return nil, output.ErrNetwork("download failed: HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) + return nil, errs.NewNetworkError(errs.SubtypeNetworkTransport, "download failed: HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) } - return nil, output.ErrNetwork("download failed: HTTP %d", resp.StatusCode) + return nil, errs.NewNetworkError(errs.SubtypeNetworkTransport, "download failed: HTTP %d", resp.StatusCode) } // resolve output path @@ -340,7 +347,7 @@ func downloadMediaFile(ctx context.Context, client *http.Client, downloadURL, mi if !opts.overwrite { if _, statErr := opts.fio.Stat(outputPath); statErr == nil { - return nil, output.ErrValidation("output file already exists: %s (use --overwrite to replace)", outputPath) + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "output file already exists: %s (use --overwrite to replace)", outputPath) } } @@ -349,7 +356,7 @@ func downloadMediaFile(ctx context.Context, client *http.Client, downloadURL, mi ContentLength: resp.ContentLength, }, resp.Body) if err != nil { - return nil, common.WrapSaveErrorByCategory(err, "io") + return nil, minutesSaveError(err) } resolvedPath, err := opts.fio.ResolvePath(outputPath) if err != nil || resolvedPath == "" { @@ -417,3 +424,21 @@ func extFromContentType(contentType string) string { } return "" } + +// minutesSaveError maps a FileIO.Save error to a typed error: path +// validation failures are validation errors; mkdir/write failures are +// internal file-I/O errors. +func minutesSaveError(err error) error { + if err == nil { + return nil + } + var me *fileio.MkdirError + switch { + case errors.Is(err, fileio.ErrPathValidation): + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithCause(err) + case errors.As(err, &me): + return errs.NewInternalError(errs.SubtypeFileIO, "cannot create parent directory: %s", err).WithCause(err) + default: + return errs.NewInternalError(errs.SubtypeFileIO, "cannot create file: %s", err).WithCause(err) + } +} diff --git a/shortcuts/minutes/minutes_download_test.go b/shortcuts/minutes/minutes_download_test.go index b4ddb888e..3a97ea629 100644 --- a/shortcuts/minutes/minutes_download_test.go +++ b/shortcuts/minutes/minutes_download_test.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "net/http" "os" "strings" @@ -15,9 +16,11 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -694,3 +697,284 @@ func TestDownload_Batch_DryRun(t *testing.T) { t.Errorf("dry-run should show tokens, got: %s", out) } } + +// --------------------------------------------------------------------------- +// Typed-error lock tests +// --------------------------------------------------------------------------- + +// TestDownload_TypedErr_ValidationInvalidArgument verifies that an invalid +// minute token format (passing cobra's required check but failing our regex) +// returns a *errs.ValidationError with SubtypeInvalidArgument and the expected +// Param. This locks site :64 (invalid minute token %q). +func TestDownload_TypedErr_ValidationInvalidArgument(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, defaultConfig()) + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-tokens", "INVALID***TOKEN", "--as", "user", + }, f, nil) + if err == nil { + t.Fatal("expected error, got nil") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--minute-tokens" { + t.Errorf("Param = %q, want %q", ve.Param, "--minute-tokens") + } +} + +// TestDownload_TypedErr_NetworkTransport_HttpError verifies that a non-2xx +// download response from downloadMediaFile returns a *errs.NetworkError with +// SubtypeNetworkTransport. +// +// In the end-to-end single-token Execute path the typed error is now passed +// through directly via r.err (single-mode passthrough). We call downloadMediaFile +// directly via a probe shortcut to assert the typed shape at the source. +func TestDownload_TypedErr_NetworkTransport_HttpError(t *testing.T) { + chdir(t, t.TempDir()) + + var capturedErr error + probe := common.Shortcut{ + Service: "minutes", + Command: "+probe-dl", + AuthTypes: []string{"bot"}, + Execute: func(ctx context.Context, rctx *common.RuntimeContext) error { + client, err := rctx.Factory.HttpClient() + if err != nil { + return err + } + _, capturedErr = downloadMediaFile(ctx, client, + "https://example.com/presigned/download", "tok001", + downloadOpts{fio: rctx.FileIO(), outputPath: "out.mp4"}) + return nil + }, + } + + f, _, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(&httpmock.Stub{ + URL: "example.com/presigned/download", + Status: 503, + RawBody: []byte("Service Unavailable"), + }) + + if err := mountAndRun(t, probe, []string{"+probe-dl", "--as", "bot"}, f, nil); err != nil { + t.Fatalf("probe shortcut should not error: %v", err) + } + if capturedErr == nil { + t.Fatal("expected downloadMediaFile to return an error for HTTP 503, got nil") + } + var ne *errs.NetworkError + if !errors.As(capturedErr, &ne) { + t.Fatalf("expected *errs.NetworkError, got %T: %v", capturedErr, capturedErr) + } + if ne.Subtype != errs.SubtypeNetworkTransport { + t.Errorf("Subtype = %q, want %q", ne.Subtype, errs.SubtypeNetworkTransport) + } + if !strings.Contains(ne.Error(), "503") { + t.Errorf("error message should contain status code 503, got: %v", ne) + } +} + +// TestDownload_TypedErr_InternalInvalidResponse verifies that fetchDownloadURL +// returns *errs.InternalError with SubtypeInvalidResponse when the API +// response contains an empty download_url field. +// +// In the end-to-end single-token Execute path the typed error is now passed +// through directly via r.err (single-mode passthrough). The typed assertion +// is also made at the fetchDownloadURL call site directly via a probe shortcut. +func TestDownload_TypedErr_InternalInvalidResponse(t *testing.T) { + var capturedErr error + probe := common.Shortcut{ + Service: "minutes", + Command: "+probe-download-url", + AuthTypes: []string{"bot"}, + Execute: func(ctx context.Context, rctx *common.RuntimeContext) error { + _, capturedErr = fetchDownloadURL(ctx, rctx, "tok001") + // Always return nil so mountAndRun doesn't swallow the error type. + return nil + }, + } + + f, _, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/minutes/v1/minutes/tok001/media", + Status: 200, + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{"download_url": ""}, + }, + }) + + if err := mountAndRun(t, probe, []string{"+probe-download-url", "--as", "bot"}, f, nil); err != nil { + t.Fatalf("probe shortcut should not error: %v", err) + } + if capturedErr == nil { + t.Fatal("expected fetchDownloadURL to return an error for empty download_url, got nil") + } + var ie *errs.InternalError + if !errors.As(capturedErr, &ie) { + t.Fatalf("expected *errs.InternalError, got %T: %v", capturedErr, capturedErr) + } + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("Subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) + } + if !strings.Contains(ie.Error(), "download_url") { + t.Errorf("error message should mention download_url, got: %v", ie) + } +} + +// TestDownload_TypedErr_OverwriteProtection verifies that the overwrite guard +// in downloadMediaFile returns *errs.ValidationError with SubtypeFailedPrecondition. +// +// In the end-to-end single-token Execute path this typed error is now passed +// through directly via r.err (single-mode passthrough), so the typed shape is +// also asserted end-to-end via the probe shortcut. +func TestDownload_TypedErr_OverwriteProtection(t *testing.T) { + chdir(t, t.TempDir()) + if err := os.WriteFile("existing.mp4", []byte("old"), 0644); err != nil { + t.Fatalf("setup failed: %v", err) + } + + var capturedErr error + probe := common.Shortcut{ + Service: "minutes", + Command: "+probe-overwrite", + AuthTypes: []string{"bot"}, + Execute: func(ctx context.Context, rctx *common.RuntimeContext) error { + client, err := rctx.Factory.HttpClient() + if err != nil { + return err + } + _, capturedErr = downloadMediaFile(ctx, client, + "https://example.com/presigned/download", "tok001", + downloadOpts{fio: rctx.FileIO(), outputPath: "existing.mp4", overwrite: false}) + return nil + }, + } + + f, _, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(downloadStub("example.com/presigned/download", []byte("new-content"), "video/mp4")) + + if err := mountAndRun(t, probe, []string{"+probe-overwrite", "--as", "bot"}, f, nil); err != nil { + t.Fatalf("probe shortcut should not error: %v", err) + } + if capturedErr == nil { + t.Fatal("expected downloadMediaFile to return an error for existing file without overwrite, got nil") + } + var ve *errs.ValidationError + if !errors.As(capturedErr, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", capturedErr, capturedErr) + } + if ve.Subtype != errs.SubtypeFailedPrecondition { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeFailedPrecondition) + } + if !strings.Contains(ve.Error(), "exists") { + t.Errorf("error message should mention exists, got: %v", ve) + } +} + +// TestDownload_TypedErr_SingleMode_PassthroughTyped verifies that in single-token +// mode a typed error from fetchDownloadURL or downloadMediaFile is returned +// directly to the caller with its Problem shape intact (exit code preserved). +func TestDownload_TypedErr_SingleMode_PassthroughTyped(t *testing.T) { + chdir(t, t.TempDir()) + + f, _, _, reg := cmdutil.TestFactory(t, defaultConfig()) + // API returns non-zero code → CallAPITyped yields a typed APIError. + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/minutes/v1/minutes/tok001/media", + Status: 200, + Body: map[string]interface{}{ + "code": 99991, "msg": "permission denied", + "data": map[string]interface{}{}, + }, + }) + + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-tokens", "tok001", "--output", "out.mp4", "--as", "bot", + }, f, nil) + if err == nil { + t.Fatal("expected error for API failure, got nil") + } + + // The error must carry a Problem (typed envelope). + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed error (ProblemOf ok), got %T: %v", err, err) + } + if p == nil { + t.Fatal("ProblemOf returned nil Problem") + } + + // Exit code must be non-zero and come from the typed error, not a generic 1. + code := output.ExitCodeOf(err) + if code == 0 { + t.Errorf("ExitCodeOf typed error = 0, want non-zero") + } +} + +// TestDownload_TypedErr_Batch_AllFail_OutPartialFailure verifies that when every +// token in a batch fails, Execute emits an ok:false stdout envelope (carrying the +// full downloads array) and returns *output.PartialFailureError with Code==ExitAPI. +// This locks the double-emit fix: the old code called OutFormat then returned ErrAPI; +// the new code calls OutPartialFailure once. +func TestDownload_TypedErr_Batch_AllFail_OutPartialFailure(t *testing.T) { + chdir(t, t.TempDir()) + + f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) + // Both tokens fail at the API level. + for _, tok := range []string{"tok001", "tok002"} { + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/minutes/v1/minutes/" + tok + "/media", + Status: 200, + Body: map[string]interface{}{ + "code": 99991, "msg": "permission denied", + "data": map[string]interface{}{}, + }, + }) + } + + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-tokens", "tok001,tok002", "--as", "bot", + }, f, stdout) + + // Must return *output.PartialFailureError with ExitAPI. + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("expected *output.PartialFailureError, got %T: %v", err, err) + } + if pfErr.Code != output.ExitAPI { + t.Errorf("PartialFailureError.Code = %d, want %d (ExitAPI)", pfErr.Code, output.ExitAPI) + } + + // stdout must carry ok:false with the downloads array (both failed entries). + var env struct { + OK bool `json:"ok"` + Data struct { + Downloads []struct { + MinuteToken string `json:"minute_token"` + Error string `json:"error"` + } `json:"downloads"` + } `json:"data"` + } + if jsonErr := json.Unmarshal(stdout.Bytes(), &env); jsonErr != nil { + t.Fatalf("failed to parse stdout: %v\nraw: %s", jsonErr, stdout.String()) + } + if env.OK { + t.Errorf("ok must be false on all-fail batch, got ok:true\nstdout: %s", stdout.String()) + } + if len(env.Data.Downloads) != 2 { + t.Fatalf("expected 2 download entries, got %d\nstdout: %s", len(env.Data.Downloads), stdout.String()) + } + for _, d := range env.Data.Downloads { + if d.Error == "" { + t.Errorf("token %s: expected non-empty error field in all-fail batch", d.MinuteToken) + } + } +} diff --git a/shortcuts/minutes/minutes_search.go b/shortcuts/minutes/minutes_search.go index 0e9a2adac..c87217925 100644 --- a/shortcuts/minutes/minutes_search.go +++ b/shortcuts/minutes/minutes_search.go @@ -13,6 +13,7 @@ import ( "time" "unicode/utf8" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -35,28 +36,28 @@ func parseTimeRange(runtime *common.RuntimeContext) (string, string, error) { if start != "" { parsed, err := toRFC3339(start) if err != nil { - return "", "", output.ErrValidation("--start: %v", err) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--start: %v", err).WithParam("--start") } startTime = parsed } if end != "" { parsed, err := toRFC3339(end, "end") if err != nil { - return "", "", output.ErrValidation("--end: %v", err) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--end: %v", err).WithParam("--end") } endTime = parsed } if startTime != "" && endTime != "" { st, err := time.Parse(time.RFC3339, startTime) if err != nil { - return "", "", fmt.Errorf("parse normalized --start: %w", err) + return "", "", errs.NewInternalError(errs.SubtypeUnknown, "parse normalized --start: %v", err).WithCause(err) } et, err := time.Parse(time.RFC3339, endTime) if err != nil { - return "", "", fmt.Errorf("parse normalized --end: %w", err) + return "", "", errs.NewInternalError(errs.SubtypeUnknown, "parse normalized --end: %v", err).WithCause(err) } if st.After(et) { - return "", "", output.ErrValidation("--start (%s) is after --end (%s)", start, end) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--start (%s) is after --end (%s)", start, end).WithParam("--start") } } return startTime, endTime, nil @@ -70,7 +71,7 @@ func toRFC3339(input string, hint ...string) (string, error) { } sec, err := strconv.ParseInt(ts, 10, 64) if err != nil { - return "", fmt.Errorf("invalid timestamp %q: %w", ts, err) + return "", fmt.Errorf("invalid timestamp %q: %w", ts, err) //nolint:forbidigo // intermediate parse error; callers wrap it into a typed ValidationError } return time.Unix(sec, 0).Format(time.RFC3339), nil } @@ -231,7 +232,7 @@ var MinutesSearch = common.Shortcut{ return err } if q := strings.TrimSpace(runtime.Str("query")); q != "" && utf8.RuneCountInString(q) > maxMinutesSearchQueryLen { - return output.ErrValidation("--query: length must be between 1 and 50 characters") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--query: length must be between 1 and 50 characters").WithParam("--query") } if _, err := common.ValidatePageSize(runtime, "page-size", defaultMinutesSearchPageSize, 1, maxMinutesSearchPageSize); err != nil { return err @@ -259,7 +260,7 @@ var MinutesSearch = common.Shortcut{ return nil } } - return common.FlagErrorf("specify at least one of --query, --owner-ids, --participant-ids, --start, or --end") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify at least one of --query, --owner-ids, --participant-ids, --start, or --end") }, DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { startTime, endTime, err := parseTimeRange(runtime) @@ -288,7 +289,7 @@ var MinutesSearch = common.Shortcut{ return err } - data, err := runtime.CallAPI(http.MethodPost, "/open-apis/minutes/v1/minutes/search", buildMinutesSearchParams(runtime), body) + data, err := runtime.CallAPITyped(http.MethodPost, "/open-apis/minutes/v1/minutes/search", buildMinutesSearchParams(runtime), body) if err != nil { return err } diff --git a/shortcuts/minutes/minutes_search_test.go b/shortcuts/minutes/minutes_search_test.go index 682368d33..d119b1ad9 100644 --- a/shortcuts/minutes/minutes_search_test.go +++ b/shortcuts/minutes/minutes_search_test.go @@ -6,9 +6,11 @@ package minutes import ( "context" "encoding/json" + "errors" "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/httpmock" @@ -63,10 +65,11 @@ func TestMinutesSearchParseTimeRangeErrors(t *testing.T) { start string end string wantMessage string + wantParam string }{ - {name: "invalid start", start: "bad-start", wantMessage: "--start:"}, - {name: "invalid end", end: "bad-end", wantMessage: "--end:"}, - {name: "start after end", start: "2026-03-26", end: "2026-03-25", wantMessage: "is after --end"}, + {name: "invalid start", start: "bad-start", wantMessage: "--start:", wantParam: "--start"}, + {name: "invalid end", end: "bad-end", wantMessage: "--end:", wantParam: "--end"}, + {name: "start after end", start: "2026-03-26", end: "2026-03-25", wantMessage: "is after --end", wantParam: "--start"}, } for _, tt := range tests { @@ -88,6 +91,16 @@ func TestMinutesSearchParseTimeRangeErrors(t *testing.T) { if !strings.Contains(err.Error(), tt.wantMessage) { t.Fatalf("error = %v, want %q", err, tt.wantMessage) } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T", err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("Subtype = %q, want SubtypeInvalidArgument", ve.Subtype) + } + if ve.Param != tt.wantParam { + t.Fatalf("Param = %q, want %q", ve.Param, tt.wantParam) + } }) } } @@ -267,6 +280,13 @@ func TestMinutesSearchValidationNoFilter(t *testing.T) { if !strings.Contains(err.Error(), "specify at least one") { t.Fatalf("unexpected error: %v", err) } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T", err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("Subtype = %q, want SubtypeInvalidArgument", ve.Subtype) + } } // TestMinutesSearchValidationInvalidParticipantID verifies participant IDs must be valid open_ids. @@ -306,6 +326,16 @@ func TestMinutesSearchValidationQueryTooLong(t *testing.T) { if !strings.Contains(err.Error(), "length must be between 1 and 50 characters") { t.Fatalf("unexpected error: %v", err) } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T", err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("Subtype = %q, want SubtypeInvalidArgument", ve.Subtype) + } + if ve.Param != "--query" { + t.Fatalf("Param = %q, want --query", ve.Param) + } } // TestMinutesSearchValidationMaxPageSize30 verifies the maximum allowed page size passes validation. @@ -372,6 +402,13 @@ func TestMinutesSearchValidationTimeErrors(t *testing.T) { if !strings.Contains(err.Error(), tt.wantMessage) { t.Fatalf("error = %v, want %q", err, tt.wantMessage) } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T", err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("Subtype = %q, want SubtypeInvalidArgument", ve.Subtype) + } }) } } diff --git a/shortcuts/minutes/minutes_speaker_replace.go b/shortcuts/minutes/minutes_speaker_replace.go index ccba0ea8e..1c28c484b 100644 --- a/shortcuts/minutes/minutes_speaker_replace.go +++ b/shortcuts/minutes/minutes_speaker_replace.go @@ -5,12 +5,11 @@ package minutes import ( "context" - "errors" "fmt" "net/http" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -37,27 +36,27 @@ var MinutesSpeakerReplace = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { minuteToken := strings.TrimSpace(runtime.Str("minute-token")) if minuteToken == "" { - return output.ErrValidation("--minute-token is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--minute-token is required").WithParam("--minute-token") } if err := validate.ResourceName(minuteToken, "--minute-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--minute-token") } fromUserID := strings.TrimSpace(runtime.Str("from-user-id")) if fromUserID == "" { - return output.ErrValidation("--from-user-id is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--from-user-id is required").WithParam("--from-user-id") } if _, err := common.ValidateUserID(fromUserID); err != nil { - return output.ErrValidation("--from-user-id: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--from-user-id: %s", err).WithParam("--from-user-id") } toUserID := strings.TrimSpace(runtime.Str("to-user-id")) if toUserID == "" { - return output.ErrValidation("--to-user-id is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--to-user-id is required").WithParam("--to-user-id") } if _, err := common.ValidateUserID(toUserID); err != nil { - return output.ErrValidation("--to-user-id: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--to-user-id: %s", err).WithParam("--to-user-id") } if fromUserID == toUserID { - return output.ErrValidation("--from-user-id and --to-user-id must be different") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--from-user-id and --to-user-id must be different").WithParam("--to-user-id") } return nil }, @@ -84,7 +83,7 @@ var MinutesSpeakerReplace = common.Shortcut{ "to_user_id": toUserID, } - _, err := runtime.CallAPI(http.MethodPut, + _, err := runtime.CallAPITyped(http.MethodPut, fmt.Sprintf("/open-apis/minutes/v1/minutes/%s/transcript/speaker", validate.EncodePathSegment(minuteToken)), nil, body) if err != nil { @@ -103,37 +102,17 @@ var MinutesSpeakerReplace = common.Shortcut{ } func minutesSpeakerReplaceError(err error, minuteToken, fromUserID string) error { - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { + p, ok := errs.ProblemOf(err) + if !ok { return err } - - switch exitErr.Detail.Code { + switch p.Code { case minutesSpeakerReplaceNoEditPermission: - return &output.ExitError{ - Code: output.ExitAPI, - Detail: &output.ErrDetail{ - Type: "no_edit_permission", - Code: minutesSpeakerReplaceNoEditPermission, - Message: fmt.Sprintf("No edit permission for minute %q: cannot replace the transcript speaker.", minuteToken), - Hint: "Ask the minute owner for minute edit permission", - Detail: exitErr.Detail.Detail, - }, - Err: err, - } + p.Message = fmt.Sprintf("No edit permission for minute %q: cannot replace the transcript speaker.", minuteToken) + p.Hint = "Ask the minute owner for minute edit permission" case minutesSpeakerReplaceSpeakerNotFoundCode: - return &output.ExitError{ - Code: output.ExitAPI, - Detail: &output.ErrDetail{ - Type: "speaker_not_found", - Code: minutesSpeakerReplaceSpeakerNotFoundCode, - Message: fmt.Sprintf("Speaker not found in minute %q: --from-user-id %q does not match an existing speaker in the transcript.", minuteToken, fromUserID), - Hint: "Check --minute-token and --from-user-id. Use an open_id for a speaker that appears in the minute transcript, then retry.", - Detail: exitErr.Detail.Detail, - }, - Err: err, - } + p.Message = fmt.Sprintf("Speaker not found in minute %q: --from-user-id %q does not match an existing speaker in the transcript.", minuteToken, fromUserID) + p.Hint = "Check --minute-token and --from-user-id. Use an open_id for a speaker that appears in the minute transcript, then retry." } - return err } diff --git a/shortcuts/minutes/minutes_speaker_replace_test.go b/shortcuts/minutes/minutes_speaker_replace_test.go index 899f15acd..787e1c1dc 100644 --- a/shortcuts/minutes/minutes_speaker_replace_test.go +++ b/shortcuts/minutes/minutes_speaker_replace_test.go @@ -10,9 +10,9 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" - "github.com/larksuite/cli/internal/output" "github.com/spf13/cobra" ) @@ -76,6 +76,52 @@ func TestMinutesSpeakerReplace_Validate(t *testing.T) { } } +func TestMinutesSpeakerReplace_ValidateTyped(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, _, _, _ := cmdutil.TestFactory(t, defaultConfig()) + + tests := []struct { + name string + args []string + wantParam string + }{ + { + name: "invalid from prefix", + args: []string{"+speaker-replace", "--minute-token", "obcn123456", "--from-user-id", "u_a", "--to-user-id", "ou_b", "--as", "user"}, + wantParam: "--from-user-id", + }, + { + name: "from equals to", + args: []string{"+speaker-replace", "--minute-token", "obcn123456", "--from-user-id", "ou_same", "--to-user-id", "ou_same", "--as", "user"}, + wantParam: "--to-user-id", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parent := &cobra.Command{Use: "minutes"} + MinutesSpeakerReplace.Mount(parent, f) + parent.SetArgs(tt.args) + parent.SilenceErrors = true + parent.SilenceUsage = true + err := parent.Execute() + if err == nil { + t.Fatalf("expected error, got nil") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("want *errs.ValidationError, got %T", err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype=%q", ve.Subtype) + } + if ve.Param != tt.wantParam { + t.Errorf("param=%q, want %q", ve.Param, tt.wantParam) + } + }) + } +} + func TestMinutesSpeakerReplace_DryRun(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, stdout, _, _ := cmdutil.TestFactory(t, defaultConfig()) @@ -179,24 +225,21 @@ func TestMinutesSpeakerReplace_SpeakerNotFound(t *testing.T) { t.Fatal("expected speaker-not-found error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("expected *output.ExitError, got %T: %v", err, err) - } - if exitErr.Detail == nil { - t.Fatalf("expected structured error detail, got nil") + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("want typed errs.*, got %T: %v", err, err) } - if exitErr.Detail.Type != "speaker_not_found" { - t.Errorf("error type = %q, want speaker_not_found", exitErr.Detail.Type) + if p.Subtype != errs.SubtypeNotFound { + t.Errorf("subtype = %q, want %q", p.Subtype, errs.SubtypeNotFound) } - if !strings.Contains(exitErr.Detail.Message, "Speaker not found") { - t.Errorf("message should be friendly, got: %s", exitErr.Detail.Message) + if !strings.Contains(p.Message, "Speaker not found") { + t.Errorf("message should be friendly, got: %s", p.Message) } - if !strings.Contains(exitErr.Detail.Message, "ou_missing_speaker") { - t.Errorf("message should include missing speaker id, got: %s", exitErr.Detail.Message) + if !strings.Contains(p.Message, "ou_missing_speaker") { + t.Errorf("message should include missing speaker id, got: %s", p.Message) } - if !strings.Contains(exitErr.Detail.Hint, "--from-user-id") { - t.Errorf("hint should mention --from-user-id, got: %s", exitErr.Detail.Hint) + if !strings.Contains(p.Hint, "--from-user-id") { + t.Errorf("hint should mention --from-user-id, got: %s", p.Hint) } } @@ -225,23 +268,20 @@ func TestMinutesSpeakerReplace_NoEditPermission(t *testing.T) { t.Fatal("expected no-edit-permission error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("expected *output.ExitError, got %T: %v", err, err) - } - if exitErr.Detail == nil { - t.Fatalf("expected structured error detail, got nil") + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("want typed errs.*, got %T: %v", err, err) } - if exitErr.Detail.Type != "no_edit_permission" { - t.Errorf("error type = %q, want no_edit_permission", exitErr.Detail.Type) + if p.Subtype != errs.SubtypePermissionDenied { + t.Errorf("subtype = %q, want %q", p.Subtype, errs.SubtypePermissionDenied) } - if !strings.Contains(exitErr.Detail.Message, "No edit permission") { - t.Errorf("message should be friendly, got: %s", exitErr.Detail.Message) + if !strings.Contains(p.Message, "No edit permission") { + t.Errorf("message should be friendly, got: %s", p.Message) } - if !strings.Contains(exitErr.Detail.Message, minutesSpeakerReplaceTestToken) { - t.Errorf("message should include minute token, got: %s", exitErr.Detail.Message) + if !strings.Contains(p.Message, minutesSpeakerReplaceTestToken) { + t.Errorf("message should include minute token, got: %s", p.Message) } - if !strings.Contains(exitErr.Detail.Hint, "edit permission") { - t.Errorf("hint should mention edit permission, got: %s", exitErr.Detail.Hint) + if !strings.Contains(p.Hint, "edit permission") { + t.Errorf("hint should mention edit permission, got: %s", p.Hint) } } diff --git a/shortcuts/minutes/minutes_update.go b/shortcuts/minutes/minutes_update.go index bdf30c680..b7fa2bf66 100644 --- a/shortcuts/minutes/minutes_update.go +++ b/shortcuts/minutes/minutes_update.go @@ -5,12 +5,11 @@ package minutes import ( "context" - "errors" "fmt" "net/http" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -33,13 +32,13 @@ var MinutesUpdate = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { minuteToken := strings.TrimSpace(runtime.Str("minute-token")) if minuteToken == "" { - return output.ErrValidation("--minute-token is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--minute-token is required").WithParam("--minute-token") } if err := validate.ResourceName(minuteToken, "--minute-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--minute-token") } if strings.TrimSpace(runtime.Str("topic")) == "" { - return output.ErrValidation("--topic is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--topic is required").WithParam("--topic") } return nil }, @@ -57,7 +56,7 @@ var MinutesUpdate = common.Shortcut{ "topic": topic, } - _, err := runtime.CallAPI(http.MethodPatch, + _, err := runtime.CallAPITyped(http.MethodPatch, fmt.Sprintf("/open-apis/minutes/v1/minutes/%s", validate.EncodePathSegment(minuteToken)), nil, body) if err != nil { @@ -75,20 +74,11 @@ var MinutesUpdate = common.Shortcut{ } func minutesUpdateError(err error, minuteToken string) error { - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil || exitErr.Detail.Code != minutesUpdateNoEditPermissionCode { + p, ok := errs.ProblemOf(err) + if !ok || p.Code != minutesUpdateNoEditPermissionCode { return err } - - return &output.ExitError{ - Code: output.ExitAPI, - Detail: &output.ErrDetail{ - Type: "no_edit_permission", - Code: minutesUpdateNoEditPermissionCode, - Message: fmt.Sprintf("No edit permission for minute %q: cannot update the title.", minuteToken), - Hint: "Ask the minute owner for minute edit permission", - Detail: exitErr.Detail.Detail, - }, - Err: err, - } + p.Message = fmt.Sprintf("No edit permission for minute %q: cannot update the title.", minuteToken) + p.Hint = "Ask the minute owner for minute edit permission" + return err } diff --git a/shortcuts/minutes/minutes_update_test.go b/shortcuts/minutes/minutes_update_test.go index c061eccf5..8c7d5e00d 100644 --- a/shortcuts/minutes/minutes_update_test.go +++ b/shortcuts/minutes/minutes_update_test.go @@ -9,9 +9,9 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" - "github.com/larksuite/cli/internal/output" "github.com/spf13/cobra" ) @@ -55,6 +55,32 @@ func TestMinutesUpdate_Validate(t *testing.T) { } } +func TestMinutesUpdate_ValidateTyped(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, _, _, _ := cmdutil.TestFactory(t, defaultConfig()) + + // ".." triggers ResourceName rejection — hits our Validate, not cobra's required-flag check. + parent := &cobra.Command{Use: "minutes"} + MinutesUpdate.Mount(parent, f) + parent.SetArgs([]string{"+update", "--minute-token", "..", "--topic", "title", "--as", "user"}) + parent.SilenceErrors = true + parent.SilenceUsage = true + err := parent.Execute() + if err == nil { + t.Fatalf("expected error, got nil") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("want *errs.ValidationError, got %T", err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype=%q", ve.Subtype) + } + if ve.Param != "--minute-token" { + t.Errorf("param=%q", ve.Param) + } +} + func TestMinutesUpdate_DryRun(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, stdout, _, _ := cmdutil.TestFactory(t, defaultConfig()) @@ -132,23 +158,20 @@ func TestMinutesUpdate_NoEditPermission(t *testing.T) { t.Fatal("expected no-edit-permission error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("expected *output.ExitError, got %T: %v", err, err) - } - if exitErr.Detail == nil { - t.Fatalf("expected structured error detail, got nil") + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("want typed errs.*, got %T: %v", err, err) } - if exitErr.Detail.Type != "no_edit_permission" { - t.Errorf("error type = %q, want no_edit_permission", exitErr.Detail.Type) + if p.Subtype != errs.SubtypePermissionDenied { + t.Errorf("subtype = %q, want %q", p.Subtype, errs.SubtypePermissionDenied) } - if !strings.Contains(exitErr.Detail.Message, "No edit permission") { - t.Errorf("message should be friendly, got: %s", exitErr.Detail.Message) + if !strings.Contains(p.Message, "No edit permission") { + t.Errorf("message should be friendly, got: %s", p.Message) } - if !strings.Contains(exitErr.Detail.Message, minutesUpdateTestToken) { - t.Errorf("message should include minute token, got: %s", exitErr.Detail.Message) + if !strings.Contains(p.Message, minutesUpdateTestToken) { + t.Errorf("message should include minute token, got: %s", p.Message) } - if !strings.Contains(exitErr.Detail.Hint, "edit permission") { - t.Errorf("hint should mention edit permission, got: %s", exitErr.Detail.Hint) + if !strings.Contains(p.Hint, "edit permission") { + t.Errorf("hint should mention edit permission, got: %s", p.Hint) } } diff --git a/shortcuts/minutes/minutes_upload.go b/shortcuts/minutes/minutes_upload.go index 06fee39e7..0f2f1f288 100644 --- a/shortcuts/minutes/minutes_upload.go +++ b/shortcuts/minutes/minutes_upload.go @@ -6,7 +6,7 @@ package minutes import ( "context" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -36,10 +36,10 @@ var MinutesUpload = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { fileToken := runtime.Str("file-token") if fileToken == "" { - return output.ErrValidation("--file-token is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file-token is required").WithParam("--file-token") } if err := validate.ResourceName(fileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } return nil }, @@ -55,7 +55,7 @@ var MinutesUpload = common.Shortcut{ "file_token": fileToken, } - data, err := runtime.CallAPI("POST", "/open-apis/minutes/v1/minutes/upload", nil, body) + data, err := runtime.CallAPITyped("POST", "/open-apis/minutes/v1/minutes/upload", nil, body) if err != nil { return err } diff --git a/shortcuts/minutes/minutes_upload_test.go b/shortcuts/minutes/minutes_upload_test.go index cc6fb93b3..26f661828 100644 --- a/shortcuts/minutes/minutes_upload_test.go +++ b/shortcuts/minutes/minutes_upload_test.go @@ -5,10 +5,12 @@ package minutes import ( "encoding/json" + "errors" "net/http" "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" "github.com/spf13/cobra" @@ -46,6 +48,31 @@ func TestMinutesUpload_Validate(t *testing.T) { } } +func TestMinutesUpload_ValidateTyped(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, defaultConfig()) + + // ".." triggers ResourceName rejection — hits our Validate, not cobra's required-flag check. + parent := &cobra.Command{Use: "minutes"} + MinutesUpload.Mount(parent, f) + parent.SetArgs([]string{"+upload", "--file-token", "..", "--as", "user"}) + parent.SilenceErrors = true + parent.SilenceUsage = true + err := parent.Execute() + if err == nil { + t.Fatalf("expected error, got nil") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("want *errs.ValidationError, got %T", err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype=%q", ve.Subtype) + } + if ve.Param != "--file-token" { + t.Errorf("param=%q", ve.Param) + } +} + func TestMinutesUpload_HelpMetadata(t *testing.T) { if len(MinutesUpload.Flags) == 0 { t.Fatal("expected file-token flag metadata") diff --git a/shortcuts/vc/vc_meeting_events.go b/shortcuts/vc/vc_meeting_events.go index dd25af93c..204c10a2a 100644 --- a/shortcuts/vc/vc_meeting_events.go +++ b/shortcuts/vc/vc_meeting_events.go @@ -14,8 +14,7 @@ import ( "time" "unicode" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -37,7 +36,7 @@ func toUnixSeconds(input string, hint ...string) (string, error) { return "", err } if _, err := strconv.ParseInt(ts, 10, 64); err != nil { - return "", fmt.Errorf("invalid timestamp %q: %w", ts, err) + return "", fmt.Errorf("invalid timestamp %q: %w", ts, err) //nolint:forbidigo // intermediate parse error; callers wrap it into a typed ValidationError } return ts, nil } @@ -134,7 +133,7 @@ func meetingEventsPageSize(runtime *common.RuntimeContext) (int, error) { } pageSize, err := strconv.Atoi(pageSizeStr) if err != nil { - return 0, common.FlagErrorf("invalid --page-size %q: must be an integer", pageSizeStr) + return 0, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --page-size %q: must be an integer", pageSizeStr).WithParam("--page-size") } if pageSize < minVCMeetingEventsPageSize { return minVCMeetingEventsPageSize, nil @@ -155,11 +154,11 @@ func meetingEventsPaginationConfig(runtime *common.RuntimeContext) (bool, int) { func validateMeetingEventsMeetingID(meetingID string) error { meetingID = strings.TrimSpace(meetingID) if meetingID == "" { - return common.FlagErrorf("--meeting-id is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--meeting-id is required").WithParam("--meeting-id") } value, err := strconv.ParseInt(meetingID, 10, 64) if err != nil || value <= 0 { - return common.FlagErrorf("--meeting-id must be a positive integer, got %q", meetingID) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--meeting-id must be a positive integer, got %q", meetingID).WithParam("--meeting-id") } return nil } @@ -176,14 +175,14 @@ func parseMeetingEventsTimeRange(runtime *common.RuntimeContext) (string, string if start != "" { parsed, err := toUnixSeconds(start) if err != nil { - return "", "", output.ErrValidation("--start: %v", err) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--start: %v", err).WithParam("--start") } startTime = parsed } if end != "" { parsed, err := toUnixSeconds(end, "end") if err != nil { - return "", "", output.ErrValidation("--end: %v", err) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--end: %v", err).WithParam("--end") } endTime = parsed } @@ -191,29 +190,30 @@ func parseMeetingEventsTimeRange(runtime *common.RuntimeContext) (string, string startValue, _ := strconv.ParseInt(startTime, 10, 64) endValue, _ := strconv.ParseInt(endTime, 10, 64) if startValue > endValue { - return "", "", output.ErrValidation("--start (%s) is after --end (%s)", start, end) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--start (%s) is after --end (%s)", start, end).WithParam("--start") } } return startTime, endTime, nil } -func buildMeetingEventsParams(runtime *common.RuntimeContext, startTime, endTime string) (larkcore.QueryParams, error) { +func buildMeetingEventsParams(runtime *common.RuntimeContext, startTime, endTime string) (map[string]interface{}, error) { pageSize, err := meetingEventsPageSize(runtime) if err != nil { return nil, err } - params := make(larkcore.QueryParams) - params.Set("meeting_id", strings.TrimSpace(runtime.Str("meeting-id"))) - params.Set("page_size", strconv.Itoa(pageSize)) + params := map[string]interface{}{ + "meeting_id": strings.TrimSpace(runtime.Str("meeting-id")), + "page_size": strconv.Itoa(pageSize), + } if pageToken := strings.TrimSpace(runtime.Str("page-token")); pageToken != "" { - params.Set("page_token", pageToken) + params["page_token"] = pageToken } if startTime != "" { - params.Set("start_time", startTime) + params["start_time"] = startTime } if endTime != "" { - params.Set("end_time", endTime) + params["end_time"] = endTime } return params, nil } @@ -225,7 +225,7 @@ func fetchMeetingEvents(ctx context.Context, runtime *common.RuntimeContext, sta } autoPaginate, pageLimit := meetingEventsPaginationConfig(runtime) if !autoPaginate { - data, err := runtime.DoAPIJSON(http.MethodGet, vcMeetingEventsAPIPath, params, nil) + data, err := runtime.CallAPITyped(http.MethodGet, vcMeetingEventsAPIPath, params, nil) if err != nil { return nil, nil, false, "", err } @@ -245,7 +245,7 @@ func fetchMeetingEvents(ctx context.Context, runtime *common.RuntimeContext, sta lastHasMore bool ) for page := 0; page < pageLimit; page++ { - data, err := runtime.DoAPIJSON(http.MethodGet, vcMeetingEventsAPIPath, params, nil) + data, err := runtime.CallAPITyped(http.MethodGet, vcMeetingEventsAPIPath, params, nil) if err != nil { return nil, nil, false, "", err } @@ -260,7 +260,7 @@ func fetchMeetingEvents(ctx context.Context, runtime *common.RuntimeContext, sta if !lastHasMore || lastPageToken == "" { break } - params.Set("page_token", lastPageToken) + params["page_token"] = lastPageToken } if lastData == nil { lastData = map[string]interface{}{} @@ -271,24 +271,11 @@ func fetchMeetingEvents(ctx context.Context, runtime *common.RuntimeContext, sta return lastData, allEvents, lastHasMore, lastPageToken, nil } -func flattenQueryParams(params larkcore.QueryParams) map[string]interface{} { +func flattenQueryParams(params map[string]interface{}) map[string]interface{} { if len(params) == 0 { return nil } - flat := make(map[string]interface{}, len(params)) - for key, values := range params { - switch len(values) { - case 0: - continue - case 1: - flat[key] = values[0] - default: - copied := make([]string, len(values)) - copy(copied, values) - flat[key] = copied - } - } - return flat + return params } func compactMeetingEvents(events []interface{}) []interface{} { diff --git a/shortcuts/vc/vc_meeting_events_test.go b/shortcuts/vc/vc_meeting_events_test.go index b998fb01b..f0cb3e0ec 100644 --- a/shortcuts/vc/vc_meeting_events_test.go +++ b/shortcuts/vc/vc_meeting_events_test.go @@ -5,6 +5,7 @@ package vc import ( "context" + "errors" "reflect" "strings" "testing" @@ -12,10 +13,10 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" "github.com/larksuite/cli/shortcuts/common" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) func newMeetingEventsRuntime() *common.RuntimeContext { @@ -323,19 +324,19 @@ func TestBuildMeetingEventsParams(t *testing.T) { if err != nil { t.Fatalf("buildMeetingEventsParams() error = %v", err) } - if got := params["meeting_id"][0]; got != "7628568141510692381" { + if got := params["meeting_id"]; got != "7628568141510692381" { t.Fatalf("meeting_id = %q, want %q", got, "7628568141510692381") } - if got := params["page_size"][0]; got != "40" { + if got := params["page_size"]; got != "40" { t.Fatalf("page_size = %q, want %q", got, "40") } - if got := params["page_token"][0]; got != "1710000000000000000" { + if got := params["page_token"]; got != "1710000000000000000" { t.Fatalf("page_token = %q, want %q", got, "1710000000000000000") } - if got := params["start_time"][0]; got != "1710000000" { + if got := params["start_time"]; got != "1710000000" { t.Fatalf("start_time = %q, want %q", got, "1710000000") } - if got := params["end_time"][0]; got != "1710003600" { + if got := params["end_time"]; got != "1710003600" { t.Fatalf("end_time = %q, want %q", got, "1710003600") } } @@ -349,7 +350,7 @@ func TestBuildMeetingEventsParams_PageSizeBelowMinClampsToMin(t *testing.T) { if err != nil { t.Fatalf("buildMeetingEventsParams() error = %v", err) } - if got := params["page_size"][0]; got != "20" { + if got := params["page_size"]; got != "20" { t.Fatalf("page_size = %q, want %q when below min", got, "20") } } @@ -363,7 +364,7 @@ func TestBuildMeetingEventsParams_PageSizeAboveMaxClampsToMax(t *testing.T) { if err != nil { t.Fatalf("buildMeetingEventsParams() error = %v", err) } - if got := params["page_size"][0]; got != "100" { + if got := params["page_size"]; got != "100" { t.Fatalf("page_size = %q, want %q when above max", got, "100") } } @@ -378,7 +379,7 @@ func TestBuildMeetingEventsParams_PageAllUsesMaxPageSize(t *testing.T) { if err != nil { t.Fatalf("buildMeetingEventsParams() error = %v", err) } - if got := params["page_size"][0]; got != "100" { + if got := params["page_size"]; got != "100" { t.Fatalf("page_size = %q, want %q when page-all is set", got, "100") } } @@ -746,22 +747,30 @@ func TestFormatTimelineOffset(t *testing.T) { } func TestFlattenQueryParams(t *testing.T) { - params := larkcore.QueryParams{ - "one": []string{"1"}, - "many": []string{"2", "3"}, - "empty": []string{}, + params := map[string]interface{}{ + "one": "1", + "many": "2", } got := flattenQueryParams(params) want := map[string]interface{}{ "one": "1", - "many": []string{"2", "3"}, + "many": "2", } if !reflect.DeepEqual(got, want) { t.Fatalf("flattenQueryParams() = %#v, want %#v", got, want) } } +func TestFlattenQueryParams_NilOnEmpty(t *testing.T) { + if got := flattenQueryParams(nil); got != nil { + t.Fatalf("flattenQueryParams(nil) = %#v, want nil", got) + } + if got := flattenQueryParams(map[string]interface{}{}); got != nil { + t.Fatalf("flattenQueryParams(empty) = %#v, want nil", got) + } +} + func TestCompactMeetingPayload_DropsOnlyEmptySlices(t *testing.T) { got := compactMeetingPayload(map[string]interface{}{ "empty_items": []interface{}{}, @@ -929,3 +938,79 @@ func TestNeedsColon(t *testing.T) { } } } + +// --------------------------------------------------------------------------- +// Typed error lock assertions +// --------------------------------------------------------------------------- + +func TestMeetingEvents_Validation_InvalidMeetingID_TypedError(t *testing.T) { + runtime := newMeetingEventsRuntime() + mustSetMeetingEventsFlag(t, runtime, "meeting-id", "not-a-number") + + err := VCMeetingEvents.Validate(context.Background(), runtime) + if err == nil { + t.Fatal("expected validation error") + } + if !strings.Contains(err.Error(), "positive integer") { + t.Errorf("message mismatch: %v", err) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--meeting-id" { + t.Errorf("Param = %q, want %q", ve.Param, "--meeting-id") + } +} + +func TestMeetingEvents_Validation_InvalidPageSize_TypedError(t *testing.T) { + runtime := newMeetingEventsRuntime() + mustSetMeetingEventsFlag(t, runtime, "meeting-id", "7628568141510692381") + mustSetMeetingEventsFlag(t, runtime, "page-size", "foo") + + err := VCMeetingEvents.Validate(context.Background(), runtime) + if err == nil { + t.Fatal("expected validation error") + } + if !strings.Contains(err.Error(), "invalid --page-size") { + t.Errorf("message mismatch: %v", err) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--page-size" { + t.Errorf("Param = %q, want %q", ve.Param, "--page-size") + } +} + +func TestMeetingEvents_Validation_StartAfterEnd_TypedError(t *testing.T) { + runtime := newMeetingEventsRuntime() + mustSetMeetingEventsFlag(t, runtime, "meeting-id", "7628568141510692381") + mustSetMeetingEventsFlag(t, runtime, "start", "200") + mustSetMeetingEventsFlag(t, runtime, "end", "100") + + err := VCMeetingEvents.Validate(context.Background(), runtime) + if err == nil { + t.Fatal("expected validation error") + } + if !strings.Contains(err.Error(), "after --end") { + t.Errorf("message mismatch: %v", err) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--start" { + t.Errorf("Param = %q, want %q", ve.Param, "--start") + } +} diff --git a/shortcuts/vc/vc_meeting_join.go b/shortcuts/vc/vc_meeting_join.go index 88db36500..453caf08f 100644 --- a/shortcuts/vc/vc_meeting_join.go +++ b/shortcuts/vc/vc_meeting_join.go @@ -10,6 +10,7 @@ import ( "regexp" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -36,7 +37,7 @@ var VCMeetingJoin = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { mn := strings.TrimSpace(runtime.Str("meeting-number")) if !validMeetingNumber(mn) { - return common.FlagErrorf("--meeting-number must be exactly 9 digits, got %q", mn) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--meeting-number must be exactly 9 digits, got %q", mn).WithParam("--meeting-number") } return nil }, @@ -48,7 +49,7 @@ var VCMeetingJoin = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { body := buildMeetingJoinBody(runtime) - data, err := runtime.DoAPIJSON("POST", "/open-apis/vc/v1/bots/join", nil, body) + data, err := runtime.CallAPITyped("POST", "/open-apis/vc/v1/bots/join", nil, body) if err != nil { return err } diff --git a/shortcuts/vc/vc_meeting_leave.go b/shortcuts/vc/vc_meeting_leave.go index 011abb348..5aa040114 100644 --- a/shortcuts/vc/vc_meeting_leave.go +++ b/shortcuts/vc/vc_meeting_leave.go @@ -9,6 +9,7 @@ import ( "io" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -26,7 +27,7 @@ var VCMeetingLeave = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("meeting-id")) == "" { - return common.FlagErrorf("--meeting-id is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--meeting-id is required").WithParam("--meeting-id") } return nil }, @@ -42,7 +43,7 @@ var VCMeetingLeave = common.Shortcut{ body := map[string]interface{}{ "meeting_id": meetingID, } - data, err := runtime.DoAPIJSON("POST", "/open-apis/vc/v1/bots/leave", nil, body) + data, err := runtime.CallAPITyped("POST", "/open-apis/vc/v1/bots/leave", nil, body) if err != nil { return err } diff --git a/shortcuts/vc/vc_meeting_test.go b/shortcuts/vc/vc_meeting_test.go index 24ee95909..2f97f65aa 100644 --- a/shortcuts/vc/vc_meeting_test.go +++ b/shortcuts/vc/vc_meeting_test.go @@ -7,11 +7,13 @@ import ( "bytes" "context" "encoding/json" + "errors" "strings" "testing" "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" "github.com/larksuite/cli/shortcuts/common" @@ -530,7 +532,67 @@ func TestMeetingLeave_Execute_APIError(t *testing.T) { if err == nil { t.Fatal("expected error for API failure") } - if !strings.Contains(err.Error(), "no permission") { - t.Errorf("error should surface API message, got: %v", err) + // code 121005 classifies to a typed permission error (no edit/view rights). + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected a typed errs.* error, got %T: %v", err, err) + } + if p.Subtype != errs.SubtypePermissionDenied { + t.Errorf("subtype = %q, want %q", p.Subtype, errs.SubtypePermissionDenied) + } +} + +// --------------------------------------------------------------------------- +// Typed error lock assertions +// --------------------------------------------------------------------------- + +func TestMeetingJoin_Validate_InvalidFormat_TypedError(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("meeting-number", "", "") + cmd.Flags().String("password", "", "") + _ = cmd.Flags().Set("meeting-number", "12345678") // 8 digits — invalid + + runtime := common.TestNewRuntimeContext(cmd, defaultConfig()) + err := VCMeetingJoin.Validate(context.Background(), runtime) + if err == nil { + t.Fatal("expected validation error") + } + if !strings.Contains(err.Error(), "9 digits") { + t.Errorf("message mismatch: %v", err) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--meeting-number" { + t.Errorf("Param = %q, want %q", ve.Param, "--meeting-number") + } +} + +func TestMeetingLeave_Validate_WhitespaceOnly_TypedError(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("meeting-id", "", "") + _ = cmd.Flags().Set("meeting-id", " ") + + runtime := common.TestNewRuntimeContext(cmd, defaultConfig()) + err := VCMeetingLeave.Validate(context.Background(), runtime) + if err == nil { + t.Fatal("expected validation error") + } + if !strings.Contains(err.Error(), "meeting-id") { + t.Errorf("message mismatch: %v", err) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--meeting-id" { + t.Errorf("Param = %q, want %q", ve.Param, "--meeting-id") } } diff --git a/shortcuts/vc/vc_notes.go b/shortcuts/vc/vc_notes.go index f5115bd72..911a8720b 100644 --- a/shortcuts/vc/vc_notes.go +++ b/shortcuts/vc/vc_notes.go @@ -23,8 +23,6 @@ import ( "strings" "time" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" "github.com/larksuite/cli/internal/auth" @@ -74,22 +72,13 @@ const ( ) func minutesReadError(err error, minuteToken string) error { - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil || exitErr.Detail.Code != minutesNoReadPermissionCode { + p, ok := errs.ProblemOf(err) + if !ok || p.Code != minutesNoReadPermissionCode { return err } - - return &output.ExitError{ - Code: output.ExitAPI, - Detail: &output.ErrDetail{ - Type: "no_read_permission", - Code: minutesNoReadPermissionCode, - Message: fmt.Sprintf("No read permission for minute %s: cannot query the minute.", minuteToken), - Hint: "Ask the minute owner for minute file read permission", - Detail: exitErr.Detail.Detail, - }, - Err: err, - } + p.Message = fmt.Sprintf("No read permission for minute %s: cannot query the minute.", minuteToken) + p.Hint = "Ask the minute owner for minute file read permission" + return err } // validMinuteToken matches the server's minute-token format and blocks any @@ -114,19 +103,19 @@ func sanitizeLogValue(s string) string { // getPrimaryCalendarID retrieves the current user's primary calendar ID. func getPrimaryCalendarID(runtime *common.RuntimeContext) (string, error) { - data, err := runtime.DoAPIJSON(http.MethodPost, "/open-apis/calendar/v4/calendars/primary", nil, nil) + data, err := runtime.CallAPITyped(http.MethodPost, "/open-apis/calendar/v4/calendars/primary", nil, nil) if err != nil { - return "", err // preserve original API error (with lark error code) + return "", err } calendars, _ := data["calendars"].([]any) if len(calendars) == 0 { - return "", output.ErrValidation("primary calendar not found") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "primary calendar not found") } first, _ := calendars[0].(map[string]any) cal, _ := first["calendar"].(map[string]any) calID, _ := cal["calendar_id"].(string) if calID == "" { - return "", output.ErrValidation("primary calendar ID is empty") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "primary calendar ID is empty") } return calID, nil } @@ -149,23 +138,23 @@ func resolveMeetingIDsFromCalendarEvent(runtime *common.RuntimeContext, instance if needNotes { body["need_meeting_notes"] = true } - data, err := runtime.DoAPIJSON(http.MethodPost, + data, err := runtime.CallAPITyped(http.MethodPost, fmt.Sprintf("/open-apis/calendar/v4/calendars/%s/events/mget_instance_relation_info", validate.EncodePathSegment(calendarID)), nil, body) if err != nil { - return nil, fmt.Errorf("failed to query event relation info: %w", err) + return nil, err } infos, _ := data["instance_relation_infos"].([]any) if len(infos) == 0 { - return nil, fmt.Errorf("no event relation info found") + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "no event relation info found") } info, _ := infos[0].(map[string]any) rawIDs, _ := info["meeting_instance_ids"].([]any) if len(rawIDs) == 0 { - return nil, fmt.Errorf("no associated video meeting for this event") + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "no associated video meeting for this event") } result := &eventRelationInfo{} @@ -293,13 +282,12 @@ func asStringSlice(v any) []string { // Other failures fall back to the raw API error description so Agents can // still parse the underlying cause. func fetchMeetingMinuteToken(runtime *common.RuntimeContext, meetingID string) (token, errMsg string) { - data, err := runtime.DoAPIJSON(http.MethodGet, + data, err := runtime.CallAPITyped(http.MethodGet, fmt.Sprintf("/open-apis/vc/v1/meetings/%s/recording", validate.EncodePathSegment(meetingID)), nil, nil) if err != nil { - var exitErr *output.ExitError - if errors.As(err, &exitErr) && exitErr.Detail != nil { - switch exitErr.Detail.Code { + if p, ok := errs.ProblemOf(err); ok { + switch p.Code { case recordingNotFoundCode: return "", "no minute file for this meeting" case recordingNoPermissionCode: @@ -328,8 +316,8 @@ func fetchMeetingMinuteToken(runtime *common.RuntimeContext, meetingID string) ( // (semicolon-separated) so Agents always see all causes at once. The // `minute_token` field is only populated on success. func fetchNoteByMeetingID(ctx context.Context, runtime *common.RuntimeContext, meetingID string) map[string]any { - data, err := runtime.DoAPIJSON(http.MethodGet, fmt.Sprintf("/open-apis/vc/v1/meetings/%s", validate.EncodePathSegment(meetingID)), - larkcore.QueryParams{"with_participants": []string{"false"}, "query_mode": []string{"0"}}, nil) + data, err := runtime.CallAPITyped(http.MethodGet, fmt.Sprintf("/open-apis/vc/v1/meetings/%s", validate.EncodePathSegment(meetingID)), + map[string]interface{}{"with_participants": "false", "query_mode": "0"}, nil) if err != nil { return map[string]any{"meeting_id": meetingID, "error": fmt.Sprintf("failed to query meeting: %v", err)} } @@ -399,13 +387,12 @@ func hasNotesPayload(m map[string]any) bool { func fetchNoteByMinuteToken(ctx context.Context, runtime *common.RuntimeContext, minuteToken string) map[string]any { errOut := runtime.IO().ErrOut - data, err := runtime.DoAPIJSON(http.MethodGet, fmt.Sprintf("/open-apis/minutes/v1/minutes/%s", validate.EncodePathSegment(minuteToken)), nil, nil) + data, err := runtime.CallAPITyped(http.MethodGet, fmt.Sprintf("/open-apis/minutes/v1/minutes/%s", validate.EncodePathSegment(minuteToken)), nil, nil) if err != nil { err = minutesReadError(err, minuteToken) result := map[string]any{"minute_token": minuteToken, "error": err.Error()} - var exitErr *output.ExitError - if errors.As(err, &exitErr) && exitErr.Detail != nil && exitErr.Detail.Hint != "" { - result["hint"] = exitErr.Detail.Hint + if p, ok := errs.ProblemOf(err); ok && p.Hint != "" { + result["hint"] = p.Hint } return result } @@ -469,7 +456,7 @@ func sanitizeDirName(title, minuteToken string) string { func fetchInlineArtifacts(runtime *common.RuntimeContext, minuteToken string, title string, result map[string]any) { errOut := runtime.IO().ErrOut fmt.Fprintf(errOut, "%s fetching AI artifacts...\n", logPrefix) - data, err := runtime.DoAPIJSON(http.MethodGet, fmt.Sprintf("/open-apis/minutes/v1/minutes/%s/artifacts", validate.EncodePathSegment(minuteToken)), nil, nil) + data, err := runtime.CallAPITyped(http.MethodGet, fmt.Sprintf("/open-apis/minutes/v1/minutes/%s/artifacts", validate.EncodePathSegment(minuteToken)), nil, nil) if err != nil { fmt.Fprintf(errOut, "%s failed to fetch AI artifacts: %v\n", logPrefix, err) return @@ -582,11 +569,10 @@ func extractDocTokens(refs []any) []string { // fetchNoteDetail retrieves note document tokens via note_id. func fetchNoteDetail(_ context.Context, runtime *common.RuntimeContext, noteID string) map[string]any { - data, err := runtime.DoAPIJSON(http.MethodGet, fmt.Sprintf("/open-apis/vc/v1/notes/%s", validate.EncodePathSegment(noteID)), nil, nil) + data, err := runtime.CallAPITyped(http.MethodGet, fmt.Sprintf("/open-apis/vc/v1/notes/%s", validate.EncodePathSegment(noteID)), nil, nil) if err != nil { - var exitErr *output.ExitError - if errors.As(err, &exitErr) && exitErr.Detail != nil && exitErr.Detail.Code == noteNoPermissionCode { - return map[string]any{"error": fmt.Sprintf("[%v]: no read permission for this meeting note", exitErr.Detail.Code)} + if p, ok := errs.ProblemOf(err); ok && p.Code == noteNoPermissionCode { + return map[string]any{"error": fmt.Sprintf("[%v]: no read permission for this meeting note", p.Code)} } return map[string]any{"error": fmt.Sprintf("failed to query note detail: %v", err)} } @@ -638,7 +624,7 @@ var VCNotes = common.Shortcut{ for _, flag := range []string{"meeting-ids", "minute-tokens", "calendar-event-ids"} { if v := runtime.Str(flag); v != "" { if ids := common.SplitCSV(v); len(ids) > maxBatchSize { - return output.ErrValidation("--%s: too many IDs (%d), maximum is %d", flag, len(ids), maxBatchSize) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--%s: too many IDs (%d), maximum is %d", flag, len(ids), maxBatchSize).WithParam("--" + flag) } } } @@ -651,7 +637,7 @@ var VCNotes = common.Shortcut{ if v := runtime.Str("minute-tokens"); v != "" { for _, token := range common.SplitCSV(v) { if !validMinuteToken.MatchString(token) { - return output.ErrValidation("invalid minute token %q: must contain only lowercase alphanumeric characters", token) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid minute token %q: must contain only lowercase alphanumeric characters", token).WithParam("--minute-tokens") } } } @@ -772,9 +758,7 @@ var VCNotes = common.Shortcut{ // all failed → return structured error if successCount == 0 && len(results) > 0 { - outData := map[string]any{"notes": results} - runtime.OutFormat(outData, &output.Meta{Count: len(results)}, nil) - return output.ErrAPI(0, fmt.Sprintf("all %d queries failed", len(results)), nil) + return runtime.OutPartialFailure(map[string]any{"notes": results}, &output.Meta{Count: len(results)}) } // output diff --git a/shortcuts/vc/vc_notes_test.go b/shortcuts/vc/vc_notes_test.go index c4490c5d2..41210cf1e 100644 --- a/shortcuts/vc/vc_notes_test.go +++ b/shortcuts/vc/vc_notes_test.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -16,9 +17,11 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -1006,11 +1009,18 @@ func TestNotes_MeetingPath_BothFail_ErrorJoinedWithSemicolon(t *testing.T) { reg.Register(meetingGetStub("m_bothfail", "")) reg.Register(recordingErrStub("m_bothfail", 121004, "data not found")) - // Two-path failure with no payload should make the batch return ErrAPI. + // Two-path failure with no payload should make the batch return OutPartialFailure. err := mountAndRun(t, VCNotes, []string{"+notes", "--meeting-ids", "m_bothfail", "--as", "user"}, f, stdout) if err == nil { t.Fatalf("expected batch failure error, got nil") } + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("expected *output.PartialFailureError, got %T: %v", err, err) + } + if pfErr.Code != output.ExitAPI { + t.Errorf("PartialFailureError.Code = %d, want ExitAPI (%d)", pfErr.Code, output.ExitAPI) + } note := extractFirstNote(t, stdout) assertNoteFieldAbsent(t, note, "minute_token") @@ -1044,6 +1054,13 @@ func TestNotes_MeetingPath_NoteNoPermission_FriendlyHint(t *testing.T) { if err == nil { t.Fatalf("expected batch failure error, got nil") } + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("expected *output.PartialFailureError, got %T: %v", err, err) + } + if pfErr.Code != output.ExitAPI { + t.Errorf("PartialFailureError.Code = %d, want ExitAPI (%d)", pfErr.Code, output.ExitAPI) + } note := extractFirstNote(t, stdout) assertNoteFieldAbsent(t, note, "note_doc_token", "minute_token") @@ -1053,3 +1070,249 @@ func TestNotes_MeetingPath_NoteNoPermission_FriendlyHint(t *testing.T) { "; ", // note + minute causes joined with semicolon ) } + +// --------------------------------------------------------------------------- +// Typed-error lock: errs.ValidationError assertions +// --------------------------------------------------------------------------- + +func TestNotes_BatchLimit_TypedValidationError(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, defaultConfig()) + ids := make([]string, 51) + for i := range ids { + ids[i] = fmt.Sprintf("m%d", i) + } + err := mountAndRun(t, VCNotes, []string{"+notes", "--meeting-ids", strings.Join(ids, ","), "--as", "user"}, f, nil) + if err == nil { + t.Fatal("expected batch limit error") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want SubtypeInvalidArgument", ve.Subtype) + } + if !strings.HasPrefix(ve.Param, "--") { + t.Errorf("Param = %q, want prefix '--'", ve.Param) + } +} + +func TestNotes_InvalidMinuteToken_TypedValidationError(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, defaultConfig()) + err := mountAndRun(t, VCNotes, []string{"+notes", "--minute-tokens", "INVALID_TOKEN!", "--as", "user"}, f, nil) + if err == nil { + t.Fatal("expected validation error for invalid minute token") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want SubtypeInvalidArgument", ve.Subtype) + } + if ve.Param != "--minute-tokens" { + t.Errorf("Param = %q, want --minute-tokens", ve.Param) + } +} + +func TestResolveMeetingIDs_NoRelationInfo_TypedValidationError(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, _, _, reg := cmdutil.TestFactory(t, defaultConfig()) + // mget returns empty instance_relation_infos + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/calendar/v4/calendars/cal_x/events/mget_instance_relation_info", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "instance_relation_infos": []interface{}{}, + }, + }, + }) + + if err := botExec(t, "no-rel-info", f, func(_ context.Context, rctx *common.RuntimeContext) error { + _, err := resolveMeetingIDsFromCalendarEvent(rctx, "evt_x", "cal_x", false) + if err == nil { + t.Fatal("expected error for empty instance_relation_infos") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeFailedPrecondition { + t.Errorf("Subtype = %q, want SubtypeFailedPrecondition", ve.Subtype) + } + if !strings.Contains(ve.Error(), "no event relation info found") { + t.Errorf("message = %q, want contains 'no event relation info found'", ve.Error()) + } + return nil + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestResolveMeetingIDs_NoMeetingIDs_TypedValidationError(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, _, _, reg := cmdutil.TestFactory(t, defaultConfig()) + // mget returns one info entry but with no meeting_instance_ids + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/calendar/v4/calendars/cal_y/events/mget_instance_relation_info", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "instance_relation_infos": []interface{}{ + map[string]interface{}{ + "instance_id": "evt_y", + "meeting_instance_ids": []interface{}{}, + }, + }, + }, + }, + }) + + if err := botExec(t, "no-meeting-ids", f, func(_ context.Context, rctx *common.RuntimeContext) error { + _, err := resolveMeetingIDsFromCalendarEvent(rctx, "evt_y", "cal_y", false) + if err == nil { + t.Fatal("expected error for empty meeting_instance_ids") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeFailedPrecondition { + t.Errorf("Subtype = %q, want SubtypeFailedPrecondition", ve.Subtype) + } + if !strings.Contains(ve.Error(), "no associated video meeting for this event") { + t.Errorf("message = %q, want contains 'no associated video meeting for this event'", ve.Error()) + } + return nil + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// --------------------------------------------------------------------------- +// Typed-error lock: enrichment via errs.ProblemOf +// --------------------------------------------------------------------------- + +// minuteGetErrStub returns an error stub for the minutes API. +func minuteGetErrStub(token string, code int, msg string) *httpmock.Stub { + return &httpmock.Stub{ + Method: "GET", + URL: "/open-apis/minutes/v1/minutes/" + token, + Body: map[string]any{"code": code, "msg": msg}, + } +} + +// TestMinutesReadError_ProblemOf_EnrichesMessage pins that minutesReadError +// mutates the typed error's Message and Hint in-place via errs.ProblemOf when +// the server returns code 2091005 (minutes no-read-permission). +func TestMinutesReadError_ProblemOf_EnrichesMessage(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(minuteGetErrStub("tokperm", minutesNoReadPermissionCode, "no permission")) + // artifactsStub not needed: we never reach it on error + + // A single minute-token that fails on a no-read-permission code still + // produces a note carrying minute_token, so the batch exits 0 with the + // enriched error surfaced inline rather than becoming an all-fail. + if err := mountAndRun(t, VCNotes, []string{"+notes", "--minute-tokens", "tokperm", "--as", "user"}, f, stdout); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // stdout carries the note with the enriched error/hint + var resp map[string]any + if parseErr := json.Unmarshal(stdout.Bytes(), &resp); parseErr != nil { + t.Fatalf("unmarshal stdout: %v\n%s", parseErr, stdout.String()) + } + data, _ := resp["data"].(map[string]any) + notes, _ := data["notes"].([]any) + if len(notes) != 1 { + t.Fatalf("expected 1 note, got %d", len(notes)) + } + note, _ := notes[0].(map[string]any) + + errMsg, _ := note["error"].(string) + if !strings.Contains(errMsg, "No read permission for minute tokperm") { + t.Errorf("error message not enriched: %q", errMsg) + } + hint, _ := note["hint"].(string) + if !strings.Contains(hint, "minute file read permission") { + t.Errorf("hint not surfaced: %q", hint) + } +} + +// TestFetchNoteDetail_NoteNoPermission_ProblemOf pins that fetchNoteDetail +// returns a friendly error map when CallAPITyped returns code 121005 and +// ProblemOf can extract it. +func TestFetchNoteDetail_NoteNoPermission_ProblemOf(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) + + // meeting.get returns note_id, note detail returns 121005 + reg.Register(meetingGetStub("m_noteperm2", "note_perm2")) + reg.Register(noteDetailErrStub("note_perm2", noteNoPermissionCode, "no permission")) + reg.Register(recordingOKStub("m_noteperm2", "https://meetings.feishu.cn/minutes/obcpermtest")) + + // note fails but minute_token succeeds → partial success (hasNotesPayload=true) + err := mountAndRun(t, VCNotes, []string{"+notes", "--meeting-ids", "m_noteperm2", "--as", "user"}, f, stdout) + if err != nil { + t.Fatalf("unexpected error (expected partial success): %v", err) + } + note := extractFirstNote(t, stdout) + + // minute_token succeeded so hasNotesPayload=true; note error still surfaced + if got := note["minute_token"]; got != "obcpermtest" { + t.Errorf("minute_token = %v, want obcpermtest", got) + } + errMsg, _ := note["error"].(string) + if !strings.Contains(errMsg, "[121005]") || !strings.Contains(errMsg, "no read permission for this meeting note") { + t.Errorf("fetchNoteDetail permission error = %q; want contains '[121005]: no read permission for this meeting note'", errMsg) + } +} + +// TestNotes_AllFailed_OutPartialFailure pins that when every item in the batch +// fails (successCount == 0), Execute returns *output.PartialFailureError with +// ExitAPI code, and stdout still carries the ok:false envelope with notes data. +func TestNotes_AllFailed_OutPartialFailure(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) + + // Both meetings have no note_id and recording returns 121004 (no minute file) + // → hasNotesPayload == false for both → successCount == 0 + reg.Register(meetingGetStub("m_fail1", "")) + reg.Register(recordingErrStub("m_fail1", 121004, "not found")) + reg.Register(meetingGetStub("m_fail2", "")) + reg.Register(recordingErrStub("m_fail2", 121004, "not found")) + + err := mountAndRun(t, VCNotes, []string{"+notes", "--meeting-ids", "m_fail1,m_fail2", "--as", "user"}, f, stdout) + if err == nil { + t.Fatal("expected batch failure error, got nil") + } + + // typed partial-failure exit signal + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("expected *output.PartialFailureError, got %T: %v", err, err) + } + if pfErr.Code != output.ExitAPI { + t.Errorf("PartialFailureError.Code = %d, want ExitAPI (%d)", pfErr.Code, output.ExitAPI) + } + + // stdout carries ok:false envelope with both failed notes + var env struct { + OK bool `json:"ok"` + Data map[string]interface{} `json:"data"` + } + if parseErr := json.Unmarshal(stdout.Bytes(), &env); parseErr != nil { + t.Fatalf("unmarshal stdout: %v\n%s", parseErr, stdout.String()) + } + if env.OK { + t.Errorf("ok must be false on all-fail, got ok:true") + } + notes, _ := env.Data["notes"].([]interface{}) + if len(notes) != 2 { + t.Fatalf("expected 2 notes in data, got %d\nstdout: %s", len(notes), stdout.String()) + } +} diff --git a/shortcuts/vc/vc_recording.go b/shortcuts/vc/vc_recording.go index 425bdebdf..9ab0e9e8a 100644 --- a/shortcuts/vc/vc_recording.go +++ b/shortcuts/vc/vc_recording.go @@ -18,6 +18,7 @@ import ( "strings" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/auth" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" @@ -55,7 +56,7 @@ func extractMinuteToken(recordingURL string) string { // fetchRecordingByMeetingID queries recording info for a single meeting. func fetchRecordingByMeetingID(_ context.Context, runtime *common.RuntimeContext, meetingID string) map[string]any { - data, err := runtime.DoAPIJSON(http.MethodGet, + data, err := runtime.CallAPITyped(http.MethodGet, fmt.Sprintf("/open-apis/vc/v1/meetings/%s/recording", validate.EncodePathSegment(meetingID)), nil, nil) if err != nil { @@ -104,7 +105,7 @@ var VCRecording = common.Shortcut{ for _, flag := range []string{"meeting-ids", "calendar-event-ids"} { if v := runtime.Str(flag); v != "" { if ids := common.SplitCSV(v); len(ids) > maxBatchSize { - return output.ErrValidation("--%s: too many IDs (%d), maximum is %d", flag, len(ids), maxBatchSize) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--%s: too many IDs (%d), maximum is %d", flag, len(ids), maxBatchSize).WithParam("--" + flag) } } } @@ -121,9 +122,11 @@ var VCRecording = common.Shortcut{ stored := auth.GetStoredToken(appID, userOpenID) if stored != nil { if missing := auth.MissingScopes(stored.Scope, required); len(missing) > 0 { - return output.ErrWithHint(output.ExitAuth, "missing_scope", - fmt.Sprintf("missing required scope(s): %s", strings.Join(missing, ", ")), - fmt.Sprintf("run `lark-cli auth login --scope \"%s\"` in the background. It blocks and outputs a verification URL — retrieve the URL and open it in a browser to complete login.", strings.Join(missing, " "))) + return errs.NewPermissionError(errs.SubtypeMissingScope, + "missing required scope(s): %s", strings.Join(missing, ", ")). + WithHint("run `lark-cli auth login --scope %q` in the background. It blocks and outputs a verification URL — retrieve the URL and open it in a browser to complete login.", strings.Join(missing, " ")). + WithMissingScopes(missing...). + WithIdentity(string(runtime.As())) } } } @@ -212,9 +215,7 @@ var VCRecording = common.Shortcut{ fmt.Fprintf(errOut, "%s done: %d total, %d succeeded, %d failed\n", recordingLogPrefix, len(results), successCount, len(results)-successCount) if successCount == 0 && len(results) > 0 { - outData := map[string]any{"recordings": results} - runtime.OutFormat(outData, &output.Meta{Count: len(results)}, nil) - return output.ErrAPI(0, fmt.Sprintf("all %d queries failed", len(results)), nil) + return runtime.OutPartialFailure(map[string]any{"recordings": results}, &output.Meta{Count: len(results)}) } outData := map[string]any{"recordings": results} diff --git a/shortcuts/vc/vc_recording_test.go b/shortcuts/vc/vc_recording_test.go index 0f5fe2cdf..28271b6eb 100644 --- a/shortcuts/vc/vc_recording_test.go +++ b/shortcuts/vc/vc_recording_test.go @@ -5,14 +5,21 @@ package vc import ( "context" + "encoding/json" + "errors" "fmt" "strings" "testing" + "time" "github.com/spf13/cobra" + keyring "github.com/zalando/go-keyring" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/auth" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -82,6 +89,16 @@ func TestRecording_BatchLimit_MeetingIDs(t *testing.T) { if !strings.Contains(err.Error(), "too many IDs") { t.Errorf("expected 'too many IDs' error, got: %v", err) } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("expected subtype %q, got %q", errs.SubtypeInvalidArgument, ve.Subtype) + } + if !strings.HasPrefix(ve.Param, "--") { + t.Errorf("expected Param to start with '--', got %q", ve.Param) + } } func TestRecording_BatchLimit_CalendarEventIDs(t *testing.T) { @@ -97,6 +114,65 @@ func TestRecording_BatchLimit_CalendarEventIDs(t *testing.T) { if !strings.Contains(err.Error(), "too many IDs") { t.Errorf("expected 'too many IDs' error, got: %v", err) } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("expected subtype %q, got %q", errs.SubtypeInvalidArgument, ve.Subtype) + } + if !strings.HasPrefix(ve.Param, "--") { + t.Errorf("expected Param to start with '--', got %q", ve.Param) + } +} + +func TestRecording_Validate_MissingScope(t *testing.T) { + keyring.MockInit() // use in-memory keyring to avoid macOS keychain popups + t.Setenv("HOME", t.TempDir()) + + cfg := defaultConfig() + // Store a token that intentionally lacks the vc:record:readonly scope. + token := &auth.StoredUAToken{ + UserOpenId: cfg.UserOpenId, + AppId: cfg.AppID, + AccessToken: "test-user-access-token", + RefreshToken: "test-refresh-token", + ExpiresAt: time.Now().Add(1 * time.Hour).UnixMilli(), + RefreshExpiresAt: time.Now().Add(24 * time.Hour).UnixMilli(), + Scope: "calendar:calendar:read", + GrantedAt: time.Now().Add(-1 * time.Hour).UnixMilli(), + } + if err := auth.SetStoredToken(token); err != nil { + t.Fatalf("SetStoredToken() error = %v", err) + } + t.Cleanup(func() { _ = auth.RemoveStoredToken(cfg.AppID, cfg.UserOpenId) }) + + f, _, _, _ := cmdutil.TestFactory(t, cfg) + err := mountAndRun(t, VCRecording, []string{"+recording", "--meeting-ids", "m001", "--as", "user"}, f, nil) + if err == nil { + t.Fatal("expected missing_scope error, got nil") + } + + var pe *errs.PermissionError + if !errors.As(err, &pe) { + t.Fatalf("expected *errs.PermissionError, got %T: %v", err, err) + } + if pe.Subtype != errs.SubtypeMissingScope { + t.Errorf("expected subtype %q, got %q", errs.SubtypeMissingScope, pe.Subtype) + } + if !strings.Contains(err.Error(), "missing required scope") { + t.Errorf("expected 'missing required scope' in message, got: %v", err) + } + found := false + for _, s := range pe.MissingScopes { + if s == "vc:record:readonly" { + found = true + break + } + } + if !found { + t.Errorf("expected MissingScopes to contain 'vc:record:readonly', got %v", pe.MissingScopes) + } } // --------------------------------------------------------------------------- @@ -698,8 +774,10 @@ func TestRecording_Execute_AllFailed_ErrorMessage(t *testing.T) { if !strings.Contains(e1, "data not found") { t.Errorf("m001 error should contain API message, got: %s", e1) } - if !strings.Contains(e2, "no permission") { - t.Errorf("m002 error should contain API message, got: %s", e2) + // code 121005 classifies to a typed permission error; the embedded + // result string surfaces the permission cause. + if !strings.Contains(e2, "permission") { + t.Errorf("m002 error should surface the permission failure, got: %s", e2) } if r1["meeting_id"] != "m001" { t.Errorf("error result should preserve meeting_id, got: %v", r1["meeting_id"]) @@ -773,3 +851,59 @@ func TestRecording_Execute_RecordingGenerating(t *testing.T) { t.Fatalf("unexpected error: %v", err) } } + +// TestRecording_Execute_AllFailed_OutPartialFailure exercises the full +// VCRecording.Execute path via mountAndRun when every query fails. +// The batch should surface as an ok:false envelope on stdout (OutPartialFailure) +// carrying the per-item failures under data.recordings, and return a typed +// *output.PartialFailureError (ExitAPI) — no legacy ErrAPI / "all N queries failed". +func TestRecording_Execute_AllFailed_OutPartialFailure(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/vc/v1/meetings/m001/recording", + Body: map[string]interface{}{"code": 121004, "msg": "data not found"}, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/vc/v1/meetings/m002/recording", + Body: map[string]interface{}{"code": 121005, "msg": "no permission"}, + }) + + err := mountAndRun(t, VCRecording, + []string{"+recording", "--meeting-ids", "m001,m002", "--format", "json", "--as", "user"}, + f, stdout) + + // 1. typed partial-failure exit signal — not a legacy ErrAPI string + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("expected *output.PartialFailureError, got %T: %v", err, err) + } + if pfErr.Code != output.ExitAPI { + t.Errorf("exit code = %d, want %d (ExitAPI)", pfErr.Code, output.ExitAPI) + } + + // 2. stdout envelope: ok:false, data.recordings carries both failed items + raw := stdout.Bytes() + var env struct { + OK bool `json:"ok"` + Data struct { + Recordings []map[string]interface{} `json:"recordings"` + } `json:"data"` + } + if jsonErr := json.Unmarshal(raw, &env); jsonErr != nil { + t.Fatalf("unmarshal stdout: %v\nraw=%s", jsonErr, string(raw)) + } + if env.OK { + t.Errorf("envelope ok must be false on all-failed batch, got ok:true\nstdout: %s", string(raw)) + } + if len(env.Data.Recordings) != 2 { + t.Fatalf("expected 2 failed items in data.recordings, got %d\nstdout: %s", len(env.Data.Recordings), string(raw)) + } + for _, rec := range env.Data.Recordings { + if rec["error"] == nil { + t.Errorf("each failed item must carry an error field; item: %v", rec) + } + } +} diff --git a/shortcuts/vc/vc_search.go b/shortcuts/vc/vc_search.go index 3c5d5579b..b99b33301 100644 --- a/shortcuts/vc/vc_search.go +++ b/shortcuts/vc/vc_search.go @@ -12,9 +12,9 @@ import ( "time" "unicode/utf8" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) const ( @@ -31,7 +31,7 @@ func toRFC3339(input string, hint ...string) (string, error) { } sec, err := strconv.ParseInt(ts, 10, 64) if err != nil { - return "", fmt.Errorf("invalid timestamp %q: %w", ts, err) + return "", fmt.Errorf("invalid timestamp %q: %w", ts, err) //nolint:forbidigo // intermediate parse error; callers wrap it into a typed ValidationError } return time.Unix(sec, 0).Format(time.RFC3339), nil } @@ -47,14 +47,14 @@ func parseTimeRange(runtime *common.RuntimeContext) (string, string, error) { if start != "" { parsed, err := toRFC3339(start) if err != nil { - return "", "", output.ErrValidation("--start: %v", err) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--start: %v", err).WithParam("--start") } startTime = parsed } if end != "" { parsed, err := toRFC3339(end, "end") if err != nil { - return "", "", output.ErrValidation("--end: %v", err) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--end: %v", err).WithParam("--end") } endTime = parsed } @@ -63,7 +63,7 @@ func parseTimeRange(runtime *common.RuntimeContext) (string, string, error) { st, _ := time.Parse(time.RFC3339, startTime) et, _ := time.Parse(time.RFC3339, endTime) if st.After(et) { - return "", "", output.ErrValidation("--start (%s) is after --end (%s)", start, end) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--start (%s) is after --end (%s)", start, end).WithParam("--start") } } return startTime, endTime, nil @@ -138,16 +138,16 @@ func buildSearchBody(runtime *common.RuntimeContext, startTime, endTime string) return body } -func buildSearchParams(runtime *common.RuntimeContext) larkcore.QueryParams { - params := larkcore.QueryParams{} +func buildSearchParams(runtime *common.RuntimeContext) map[string]interface{} { + params := map[string]interface{}{} pageToken := strings.TrimSpace(runtime.Str("page-token")) pageSize, _ := strconv.Atoi(strings.TrimSpace(runtime.Str("page-size"))) if pageSize <= 0 { pageSize = defaultVCSearchPageSize } - params["page_size"] = []string{strconv.Itoa(pageSize)} + params["page_size"] = strconv.Itoa(pageSize) if pageToken != "" { - params["page_token"] = []string{pageToken} + params["page_token"] = pageToken } return params } @@ -192,7 +192,7 @@ var VCSearch = common.Shortcut{ return err } if q := strings.TrimSpace(runtime.Str("query")); q != "" && utf8.RuneCountInString(q) > maxVCSearchQueryLen { - return output.ErrValidation("--query: length must be between 1 and 50 characters") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--query: length must be between 1 and 50 characters").WithParam("--query") } if _, err := common.ValidatePageSize(runtime, "page-size", defaultVCSearchPageSize, 1, maxVCSearchPageSize); err != nil { return err @@ -202,7 +202,7 @@ var VCSearch = common.Shortcut{ return nil } } - return common.FlagErrorf("specify at least one of --query, --start, --end, --organizer-ids, --participant-ids, or --room-ids") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify at least one of --query, --start, --end, --organizer-ids, --participant-ids, or --room-ids") }, DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { startTime, endTime, err := parseTimeRange(runtime) @@ -210,20 +210,10 @@ var VCSearch = common.Shortcut{ return common.NewDryRunAPI().Set("error", err.Error()) } params := buildSearchParams(runtime) - dryRunParams := map[string]interface{}{} - for key, values := range params { - if len(values) == 1 { - dryRunParams[key] = values[0] - } else if len(values) > 1 { - vs := make([]string, len(values)) - copy(vs, values) - dryRunParams[key] = vs - } - } dryRun := common.NewDryRunAPI(). POST("/open-apis/vc/v1/meetings/search") - if len(dryRunParams) > 0 { - dryRun.Params(dryRunParams) + if len(params) > 0 { + dryRun.Params(params) } return dryRun.Body(buildSearchBody(runtime, startTime, endTime)) }, @@ -232,7 +222,7 @@ var VCSearch = common.Shortcut{ if err != nil { return err } - data, err := runtime.DoAPIJSON("POST", "/open-apis/vc/v1/meetings/search", buildSearchParams(runtime), buildSearchBody(runtime, startTime, endTime)) + data, err := runtime.CallAPITyped("POST", "/open-apis/vc/v1/meetings/search", buildSearchParams(runtime), buildSearchBody(runtime, startTime, endTime)) if err != nil { return err } diff --git a/shortcuts/vc/vc_search_test.go b/shortcuts/vc/vc_search_test.go index 03ef60e3e..66c2ba8b3 100644 --- a/shortcuts/vc/vc_search_test.go +++ b/shortcuts/vc/vc_search_test.go @@ -5,11 +5,13 @@ package vc import ( "context" + "errors" "strings" "testing" "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/shortcuts/common" @@ -259,3 +261,182 @@ func TestSearch_InvalidTimeRange(t *testing.T) { t.Fatal("expected error for invalid time") } } + +// --------------------------------------------------------------------------- +// Typed error envelope assertions (errs migration lock-in) +// --------------------------------------------------------------------------- + +func TestParseTimeRange_InvalidStart_TypedError(t *testing.T) { + cfg := &core.CliConfig{AppID: "test", AppSecret: "s", Brand: core.BrandFeishu} + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("start", "", "") + cmd.Flags().String("end", "", "") + _ = cmd.Flags().Set("start", "not-a-date") + runtime := common.TestNewRuntimeContext(cmd, cfg) + + _, _, err := parseTimeRange(runtime) + if err == nil { + t.Fatal("expected error for invalid --start") + } + if !strings.Contains(err.Error(), "--start:") { + t.Errorf("message should contain '--start:', got: %v", err) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--start" { + t.Errorf("Param = %q, want \"--start\"", ve.Param) + } +} + +func TestParseTimeRange_InvalidEnd_TypedError(t *testing.T) { + cfg := &core.CliConfig{AppID: "test", AppSecret: "s", Brand: core.BrandFeishu} + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("start", "", "") + cmd.Flags().String("end", "", "") + _ = cmd.Flags().Set("end", "not-a-date") + runtime := common.TestNewRuntimeContext(cmd, cfg) + + _, _, err := parseTimeRange(runtime) + if err == nil { + t.Fatal("expected error for invalid --end") + } + if !strings.Contains(err.Error(), "--end:") { + t.Errorf("message should contain '--end:', got: %v", err) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--end" { + t.Errorf("Param = %q, want \"--end\"", ve.Param) + } +} + +func TestParseTimeRange_StartAfterEnd_TypedError(t *testing.T) { + cfg := &core.CliConfig{AppID: "test", AppSecret: "s", Brand: core.BrandFeishu} + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("start", "", "") + cmd.Flags().String("end", "", "") + _ = cmd.Flags().Set("start", "2026-03-25T00:00+08:00") + _ = cmd.Flags().Set("end", "2026-03-24T00:00+08:00") + runtime := common.TestNewRuntimeContext(cmd, cfg) + + _, _, err := parseTimeRange(runtime) + if err == nil { + t.Fatal("expected error for start after end") + } + if !strings.Contains(err.Error(), "--start") || !strings.Contains(err.Error(), "--end") { + t.Errorf("message should mention --start and --end, got: %v", err) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--start" { + t.Errorf("Param = %q, want \"--start\"", ve.Param) + } +} + +func TestSearch_Validation_QueryTooLong_TypedError(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("query", "", "") + cmd.Flags().String("start", "", "") + cmd.Flags().String("end", "", "") + cmd.Flags().String("organizer-ids", "", "") + cmd.Flags().String("participant-ids", "", "") + cmd.Flags().String("room-ids", "", "") + cmd.Flags().String("page-size", "", "") + _ = cmd.Flags().Set("query", strings.Repeat("x", 51)) + + runtime := common.TestNewRuntimeContext(cmd, defaultConfig()) + err := VCSearch.Validate(context.Background(), runtime) + if err == nil { + t.Fatal("expected validation error for overlong query") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--query" { + t.Errorf("Param = %q, want \"--query\"", ve.Param) + } +} + +func TestSearch_Validation_NoFilter_TypedError(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, defaultConfig()) + err := mountAndRun(t, VCSearch, []string{"+search", "--as", "user"}, f, nil) + if err == nil { + t.Fatal("expected validation error for no filter") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } +} + +func TestBuildSearchParams(t *testing.T) { + cfg := &core.CliConfig{AppID: "test", AppSecret: "s", Brand: core.BrandFeishu} + + t.Run("defaults", func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("page-token", "", "") + cmd.Flags().String("page-size", "", "") + runtime := common.TestNewRuntimeContext(cmd, cfg) + params := buildSearchParams(runtime) + if params["page_size"] != "15" { + t.Errorf("page_size = %v, want \"15\"", params["page_size"]) + } + if _, ok := params["page_token"]; ok { + t.Error("page_token should be absent when not set") + } + }) + + t.Run("custom page-size and page-token", func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("page-token", "", "") + cmd.Flags().String("page-size", "", "") + _ = cmd.Flags().Set("page-size", "20") + _ = cmd.Flags().Set("page-token", "tok123") + runtime := common.TestNewRuntimeContext(cmd, cfg) + params := buildSearchParams(runtime) + if params["page_size"] != "20" { + t.Errorf("page_size = %v, want \"20\"", params["page_size"]) + } + if params["page_token"] != "tok123" { + t.Errorf("page_token = %v, want \"tok123\"", params["page_token"]) + } + }) + + t.Run("values are scalars not slices", func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("page-token", "", "") + cmd.Flags().String("page-size", "", "") + _ = cmd.Flags().Set("page-size", "10") + _ = cmd.Flags().Set("page-token", "p") + runtime := common.TestNewRuntimeContext(cmd, cfg) + params := buildSearchParams(runtime) + if _, isSlice := params["page_size"].([]string); isSlice { + t.Error("page_size must be a scalar string, not []string") + } + if _, isSlice := params["page_token"].([]string); isSlice { + t.Error("page_token must be a scalar string, not []string") + } + }) +}