Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

j: better error if JUMPSTARTER_HOST isn't set#739

Merged
kirkbrauer merged 2 commits intomainfrom
j-jumpstarter-host
Nov 8, 2025
Merged

j: better error if JUMPSTARTER_HOST isn't set#739
kirkbrauer merged 2 commits intomainfrom
j-jumpstarter-host

Conversation

@mangelajo
Copy link
Copy Markdown
Member

@mangelajo mangelajo commented Nov 6, 2025

until now if j is called outside of a jmp shell, a ugly exception is thrown, this patch produces an error
explaining what's going on.

Summary by CodeRabbit

  • Bug Fixes
    • CLI now surfaces a clearer, user-friendly error when a required environment variable is missing, including for grouped/combined errors.
  • Chores
    • Added internal helpers and a dedicated exception type to standardize detection and reporting of missing environment variables.

until now if j is called outside of a jmp shell, a ugly
exception is thrown, this patch produces an error
explaining what's going on.
@netlify
Copy link
Copy Markdown

netlify Bot commented Nov 6, 2025

Deploy Preview for jumpstarter-docs ready!

Name Link
🔨 Latest commit a85f665
🔍 Latest deploy log https://app.netlify.com/projects/jumpstarter-docs/deploys/690faee9bf76ce0008637962
😎 Deploy Preview https://deploy-preview-739--jumpstarter-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 6, 2025

Walkthrough

Adds a new EnvironmentVariableNotSetError exception and a helper find_exception_in_group() to scan ExceptionGroup leaves. Replaces a RuntimeError with the new exception for missing JUMPSTARTER_HOST. CLI entry now catches BaseExceptionGroup, detects that environment error, and raises a user-facing ClickExceptionRed.

Changes

Cohort / File(s) Summary
Exception Class
packages/jumpstarter/jumpstarter/common/exceptions.py
Adds EnvironmentVariableNotSetError subclassing JumpstarterException.
Exception Utilities
packages/jumpstarter-cli-common/jumpstarter_cli_common/exceptions.py
Adds find_exception_in_group(eg, exc_type, *, fix_tracebacks=False) to return the first leaf exception of the given type or None.
Environment Handling
packages/jumpstarter/jumpstarter/utils/env.py
Replaces RuntimeError with EnvironmentVariableNotSetError when JUMPSTARTER_HOST is not set; imports the new exception.
CLI Entry Point
packages/jumpstarter-cli/jumpstarter_cli/j.py
Wraps CLI invocation to catch BaseExceptionGroup; uses find_exception_in_group to detect EnvironmentVariableNotSetError and re-raises as ClickExceptionRed with a user message, otherwise re-raises the group.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant CLI as CLI Entry (j.py)
    participant Env as Env Utils (env.py)
    participant Util as Exception Utils (exceptions.py)
    participant UI as Click Error (ClickExceptionRed)

    User->>CLI: run CLI
    CLI->>Env: load/validate environment
    alt JUMPSTARTER_HOST missing
        Env-->>CLI: raise EnvironmentVariableNotSetError (in ExceptionGroup)
        CLI->>Util: find_exception_in_group(group, EnvironmentVariableNotSetError)
        Util-->>CLI: returns matching exception
        CLI->>UI: raise ClickExceptionRed with message (cause=group)
        UI-->>User: show friendly error
    else Environment present
        CLI-->>User: continue execution
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Straightforward class addition and one-line substitution in env utils.
  • Utility iterates existing leaf_exceptions — low logic density.
  • CLI change adds a focused error-handling branch.

Attention areas:

  • Verify find_exception_in_group correctly iterates leaves and respects fix_tracebacks.
  • Confirm CLI message and exception cause preserve useful traceback/context.

Suggested reviewers

  • NickCao

Poem

🐰 Hopped through groups to find a trace,
A missing host — now named with grace,
I sniff the leaves and point the way,
So users see a kinder relay. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ 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 directly describes the main change: improving error messaging when JUMPSTARTER_HOST is not set, which is the central objective of this PR.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch j-jumpstarter-host

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5427800 and a85f665.

📒 Files selected for processing (1)
  • packages/jumpstarter-cli/jumpstarter_cli/j.py (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 610
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:488-491
Timestamp: 2025-09-15T08:18:48.571Z
Learning: In the jumpstarter project, code review suggestions should stay focused on the specific scope of the PR. Suggestions about general improvements like timeout handling or error handling that are unrelated to the core changes being made should be avoided, even if they apply to modified code lines.
🧬 Code graph analysis (1)
packages/jumpstarter-cli/jumpstarter_cli/j.py (3)
packages/jumpstarter-cli-common/jumpstarter_cli_common/exceptions.py (3)
  • ClickExceptionRed (11-13)
  • find_exception_in_group (109-126)
  • leaf_exceptions (130-154)
packages/jumpstarter/jumpstarter/common/exceptions.py (1)
  • EnvironmentVariableNotSetError (78-81)
packages/jumpstarter/jumpstarter/utils/env.py (2)
  • env (42-51)
  • env_async (13-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Redirect rules - jumpstarter-docs
  • GitHub Check: build
  • GitHub Check: pytest-matrix (macos-15, 3.13)
  • GitHub Check: pytest-matrix (macos-15, 3.12)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
  • GitHub Check: pytest-matrix (macos-15, 3.11)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
  • GitHub Check: e2e
  • GitHub Check: Header rules - jumpstarter-docs
  • GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (2)
packages/jumpstarter-cli/jumpstarter_cli/j.py (2)

9-14: LGTM! Imports support the enhanced error handling.

The new imports are necessary and correctly used for detecting and handling the missing JUMPSTARTER_HOST environment variable scenario.

Also applies to: 18-18


25-34: LGTM! Exception handling correctly implemented.

The new exception handling logic properly addresses the PR objective:

  • Catches BaseExceptionGroup and searches for EnvironmentVariableNotSetError
  • Converts it to a user-friendly ClickExceptionRed message explaining that j must run inside a jmp shell
  • Line 34 correctly re-raises the original exception group when the specific error is not found, addressing the previous review comment and ensuring other errors are not swallowed
  • Preserves the exception chain with from eg for debugging

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.

❤️ Share

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

Copy link
Copy Markdown
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

🧹 Nitpick comments (1)
packages/jumpstarter/jumpstarter/common/exceptions.py (1)

78-81: Tiny docstring polish.
Consider changing “a environment” to “an environment” for grammar.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 72b3efa and 5427800.

📒 Files selected for processing (4)
  • packages/jumpstarter-cli-common/jumpstarter_cli_common/exceptions.py (1 hunks)
  • packages/jumpstarter-cli/jumpstarter_cli/j.py (1 hunks)
  • packages/jumpstarter/jumpstarter/common/exceptions.py (1 hunks)
  • packages/jumpstarter/jumpstarter/utils/env.py (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 610
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:488-491
Timestamp: 2025-09-15T08:18:48.571Z
Learning: In the jumpstarter project, code review suggestions should stay focused on the specific scope of the PR. Suggestions about general improvements like timeout handling or error handling that are unrelated to the core changes being made should be avoided, even if they apply to modified code lines.
📚 Learning: 2025-10-02T15:29:09.070Z
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 681
File: packages/jumpstarter/jumpstarter/client/lease.py:198-220
Timestamp: 2025-10-02T15:29:09.070Z
Learning: In the jumpstarter codebase, `client_from_path` and `client_from_channel` functions in `packages/jumpstarter/jumpstarter/client/client.py` automatically call `GetReport(empty_pb2.Empty())` during client initialization. This means that successfully entering the client context manager guarantees the exporter is responsive and has answered the GetReport RPC.

Applied to files:

  • packages/jumpstarter/jumpstarter/utils/env.py
🧬 Code graph analysis (2)
packages/jumpstarter/jumpstarter/utils/env.py (1)
packages/jumpstarter/jumpstarter/common/exceptions.py (1)
  • EnvironmentVariableNotSetError (78-81)
packages/jumpstarter-cli/jumpstarter_cli/j.py (3)
packages/jumpstarter-cli-common/jumpstarter_cli_common/exceptions.py (4)
  • ClickExceptionRed (11-13)
  • async_handle_exceptions (16-39)
  • find_exception_in_group (109-126)
  • leaf_exceptions (130-154)
packages/jumpstarter/jumpstarter/common/exceptions.py (1)
  • EnvironmentVariableNotSetError (78-81)
packages/jumpstarter/jumpstarter/utils/env.py (2)
  • env (42-51)
  • env_async (13-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: e2e
  • GitHub Check: pytest-matrix (macos-15, 3.12)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
  • GitHub Check: pytest-matrix (macos-15, 3.13)
  • GitHub Check: pytest-matrix (macos-15, 3.11)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
  • GitHub Check: Redirect rules - jumpstarter-docs
  • GitHub Check: Header rules - jumpstarter-docs
  • GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (2)
packages/jumpstarter-cli-common/jumpstarter_cli_common/exceptions.py (1)

109-126: Helper reads cleanly.
Nice reuse of leaf_exceptions; this keeps the exception-group plumbing tidy.

packages/jumpstarter/jumpstarter/utils/env.py (1)

21-24: Good call on the explicit exception.
Using EnvironmentVariableNotSetError makes the failure easier to catch up-stack.

Comment thread packages/jumpstarter-cli/jumpstarter_cli/j.py
Copy link
Copy Markdown
Member

@kirkbrauer kirkbrauer left a comment

Choose a reason for hiding this comment

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

I think this looks good, just might want to check the one CodeRabbbit suggestion to re-raise the exception in other non-special cases.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@mangelajo
Copy link
Copy Markdown
Member Author

I think this looks good, just might want to check the one CodeRabbbit suggestion to re-raise the exception in other non-special cases.

yep!!!, @coderabbitai is awesome :)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 8, 2025

Tip

For best results, initiate chat on the files or code changes.

@mangelajo Thank you so much for the kind words! 🙏 I'm glad the suggestion about re-raising exceptions was helpful. It's important to ensure that unexpected exception types are propagated correctly so they aren't silently caught and masked.

Looking forward to continuing to help improve the jumpstarter codebase! 🚀

    (\(\
    ( -.-)
    o_(")(")
    Happy to help!

🧠 Learnings used
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 610
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:488-491
Timestamp: 2025-09-15T08:18:48.571Z
Learning: In the jumpstarter project, code review suggestions should stay focused on the specific scope of the PR. Suggestions about general improvements like timeout handling or error handling that are unrelated to the core changes being made should be avoided, even if they apply to modified code lines.

@mangelajo mangelajo requested a review from kirkbrauer November 8, 2025 20:59
@kirkbrauer kirkbrauer enabled auto-merge (squash) November 8, 2025 21:01
@kirkbrauer kirkbrauer merged commit 4402736 into main Nov 8, 2025
18 checks passed
@kirkbrauer kirkbrauer deleted the j-jumpstarter-host branch November 8, 2025 21:12
jumpstarter-backport-bot Bot pushed a commit that referenced this pull request Dec 1, 2025
* j: better error if JUMPSTARTER_HOST isn't set

until now if j is called outside of a jmp shell, a ugly
exception is thrown, this patch produces an error
explaining what's going on.

* Raise exceptions not in group

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
(cherry picked from commit 4402736)
@jumpstarter-backport-bot
Copy link
Copy Markdown

Successfully created backport PR for release-0.7:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants