fix(apps): friendly-ize "Container not exists" for observability commands - #2302
Merged
Conversation
…ands
+metric-list and +analytics-list passed the upstream business code 400002655
("Container not exists") through verbatim. The message reads like an
infrastructure fault and misleads callers (including AI agents) into retrying a
non-retryable, expected business state: an app with no running container simply
has no metrics/analytics to query yet.
Rewrite it at a scoped observability helper (withObservabilityHint) into a
user-facing explanation plus a deploy-then-retry next step, mirroring the
existing isAppNoDatabaseError override. Detection is code-OR-message so a server
renumber alone does not silently drop the rewrite. Classification, code, and the
wrapped cause are preserved; unrelated failures still fall through to the shared
app-id recovery hint (and its own no-database override).
github-actions
Bot
added
the
size/M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughApps analytics and metric-list requests now use observability-specific error hints. Missing-container failures are detected by code or message, rewritten with deployment guidance, and tested while preserving error classification and causes. ChangesObservability error hints
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/apps/apps_analytics.go`:
- Line 66: Add HTTP-mocked command-level regression tests for both wrapper
changes: in shortcuts/apps/apps_analytics.go lines 66-66, cover +analytics-list,
and in shortcuts/apps/apps_metrics.go lines 71-71, cover +metric-list. Execute
each command against a typed no-container failure and assert the rewritten
message, hint, category, subtype, code, and wrapped cause so the tests fail if
either call site reverts to withAppsHint.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4c3dc112-29aa-448b-9189-81ab92834728
📒 Files selected for processing (4)
shortcuts/apps/apps_analytics.goshortcuts/apps/apps_metrics.goshortcuts/apps/common.goshortcuts/apps/common_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@9dcd750d572620806e12a71713b8af27e55f2a00🧩 Skill updatenpx skills add larksuite/cli#fix/apps-metric-container-friendly -y -g |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2302 +/- ##
==========================================
- Coverage 76.51% 76.44% -0.08%
==========================================
Files 1019 1025 +6
Lines 112614 113679 +1065
==========================================
+ Hits 86172 86898 +726
- Misses 19873 20109 +236
- Partials 6569 6672 +103 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…tainer hint common_test.go proves withObservabilityHint in isolation but stays green if a call site reverts to withAppsHint. Drive +metric-list and +analytics-list Execute with a mocked 400002655 "Container not exists" envelope and assert the container-specific message/hint/code, so a revert fails the build. Also closes the two uncovered call-site lines flagged by coverage.
…dition Register 400002655 in sparkCodeMeta mirroring its no-database twin (400002465) so both "expected precondition not met" business states expose the same validation/failed_precondition classification to machine consumers, instead of falling back to api/unknown. The shortcut-layer message rewrite already keyed off the raw code, so this only aligns the typed envelope's category/subtype; update the execution-path tests to pin the new classification.
…tion The no-container hint told a harness to deploy via +release-create, a "write" that takes the whole app live and can affect existing production traffic — without the user-confirmation gate its no-database twin deliberately carries. Since the hint's audience is an AI agent that acts on it, a failed metrics read could trigger an unconfirmed go-live. Lead with a read-only +release-list status check and gate +release-create behind an explicit user confirmation, mirroring appNoDatabaseHint.
liangshuo-1
approved these changes
Aug 12, 2026
Merged
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apps +metric-listand+analytics-listpassed the upstream business code 400002655 ("Container not exists") through verbatim. The message reads like an infrastructure fault and misleads callers (including AI agents) into retrying a non-retryable, expected business state: an app with no running container simply has no metrics/analytics to query until it is deployed and serving traffic.Changes
withObservabilityHinthelper inshortcuts/apps/common.gothat rewrites the "no running container" business state (code400002655, or acontainer not existmessage fallback) into a user-facing explanation plus a deploy-then-retry hint, mirroring the existingisAppNoDatabaseErroroverride.apps_metrics.go,apps_analytics.go) through it instead of the rawwithAppsHint(err, appIDListHint).withAppsHintchokepoint) because 400002655 is not known to be observability-exclusive and the message-marker fallback must not reach unrelated commands.Test Plan
go test ./shortcuts/apps/(full package) and./internal/errclass/...TestWithObservabilityHint(8 subcases: code hit, message fallback, case-insensitivity, cause preservation, overrides upstream hint, unrelated failure falls through, no-database still applies) +TestIsAppNoContainerError_NilProblemgo build ./...andgo vet ./shortcuts/apps/...cleanProblem.Code).Summary by CodeRabbit