Skip to content

Comments

avoid ignoring missing stack error#1835

Merged
jordanstephens merged 5 commits intomainfrom
jordan/return-missing-stack-error
Jan 21, 2026
Merged

avoid ignoring missing stack error#1835
jordanstephens merged 5 commits intomainfrom
jordan/return-missing-stack-error

Conversation

@jordanstephens
Copy link
Member

@jordanstephens jordanstephens commented Jan 21, 2026

Description

I tried to run

defang cd cancel --project-name=html-css-js --stack=smoketestaws --workspace=DefangLabs

And got back an error that said

the CD command is not valid for the Defang playground; did you forget --stack or --provider?

Looking more closely, it looks like we swallowed the missing stack error. This PR restores it.

Linked Issues

Checklist

  • I have performed a self-review of my code
  • I have added appropriate tests
  • I have updated the Defang CLI docs and/or README to reflect my changes, if necessary

Summary by CodeRabbit

  • Bug Fixes

    • Clearer, specific error when a specified stack does not exist and improved fallback messaging when no default stack is found.
    • Loading now returns immediately for explicitly missing stacks; provider fallback behavior adjusted to ensure a consistent provider value in failure paths.
  • Tests

    • Added and updated tests to cover specified non-existent stack scenarios and the updated missing-stack error messaging.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

📝 Walkthrough

Walkthrough

Adds a concrete stacks.ErrNotExist error returned when a requested stack is missing; session stack-loading now detects that error via errors.As and returns it immediately for specified stacks. Tests updated to expect the new error form and mock behavior adjusted accordingly.

Changes

Cohort / File(s) Summary
Stacks manager
src/pkg/stacks/manager.go
Adds public ErrNotExist type (StackName string) and Error() method; surfaces ErrNotExist when a specified/remote stack is missing and adjusts fallback messaging.
Stacks tests
src/pkg/stacks/manager_test.go
Updates test expectations to the new specific missing-stack error message (e.g., stack "missingstack" does not exist).
Session loader
src/pkg/session/session.go
Imports errors; SessionLoader.loadStack uses errors.As to detect *stacks.ErrNotExist for specified missing stacks and returns it immediately; preserves other error/fallback paths and ProviderAuto -> ProviderDefang adjustment.
Session tests
src/pkg/session/session_test.go
Adds "specified non-existing stack" test case; mocks GetStack to return stacks.ErrNotExist when a non-empty stack is specified, and a generic error when no stack specified.

Sequence Diagram(s)

(omitted — changes are localized to error type and handler and do not introduce a multi-component sequential flow requiring visualization)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Lio/fix default stack #1820: Related stack-retrieval and fallback changes that touch stack-not-found handling and session↔stacks-manager interactions.

Suggested reviewers

  • jordanstephens

Poem

🐇 I hopped through code and sniffed the nest,
A missing stack put me to test.
Now ErrNotExist chirps clear and bright,
No blind guesses in the night —
I twitch my nose; the fix feels right. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: introducing specific error handling for missing stacks instead of silently ignoring them.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter"
level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/pkg/stacks/manager.go`:
- Around line 228-231: Do not blanket-convert all errors from sm.GetRemote into
ErrNotExist; instead, call stack, err = sm.GetRemote(ctx, name) and if err !=
nil inspect it (using errors.Is/As against the remote-not-found sentinel/type
returned by sm.GetRemote or by checking a dedicated IsNotFound helper) and only
return &ErrNotExist{StackName: name} when the remote error explicitly indicates
the stack is missing; otherwise propagate or wrap the original err (e.g., return
nil, "", fmt.Errorf("GetRemote %s: %w", name, err)) so network/auth/fabric
errors are preserved.

@jordanstephens jordanstephens merged commit 2d31aa1 into main Jan 21, 2026
14 checks passed
@jordanstephens jordanstephens deleted the jordan/return-missing-stack-error branch January 21, 2026 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants