Improve auth setup project-creation error handling and retry UX#95
Improve auth setup project-creation error handling and retry UX#95mkh09353 wants to merge 2 commits intogoogleworkspace:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 0b53fa6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience during the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
mkh09353
force-pushed
the
fix/setup-project-create-error-recovery
branch
2 times, most recently
from
542bae0 to
ea5ed6b
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the user experience of the gws auth setup command by enhancing error handling during GCP project creation. It introduces a retry loop, allowing users to correct invalid project IDs without restarting the setup wizard, and provides more specific, actionable guidance for common failure scenarios like unaccepted Terms of Service or invalid/used project IDs. The changes are well-structured and include corresponding unit tests. I've suggested a small refinement to the error classification to make the user-facing messages even more accurate.
| lower.contains("argument project_id: bad value") | ||
| || lower.contains("project ids are immutable") | ||
| || lower.contains("project ids must be between 6 and 30 characters") |
There was a problem hiding this comment.
The error condition project ids are immutable is not a project ID format validation error. It's more related to the ID being in a reserved state (e.g., from a recently deleted project). Grouping this with format validation errors can lead to a confusing message for the user. This check should be moved to is_project_id_in_use_error to provide more accurate feedback.
| lower.contains("argument project_id: bad value") | |
| || lower.contains("project ids are immutable") | |
| || lower.contains("project ids must be between 6 and 30 characters") | |
| lower.contains("argument project_id: bad value") | |
| || lower.contains("project ids must be between 6 and 30 characters") |
| lower.contains("already in use") | ||
| || lower.contains("already exists") | ||
| || lower.contains("already being used") |
There was a problem hiding this comment.
To provide a more accurate error message, the project ids are immutable check (moved from is_invalid_project_id_error) should be included here. This error is more akin to the project ID being already in use or reserved.
lower.contains("already in use")
|| lower.contains("already exists")
|| lower.contains("already being used")
|| lower.contains("project ids are immutable")
mkh09353
force-pushed
the
fix/setup-project-create-error-recovery
branch
from
f70d1a8 to
b4de4ab
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #95 +/- ##
==========================================
+ Coverage 54.95% 55.00% +0.04%
==========================================
Files 37 37
Lines 12218 12407 +189
==========================================
+ Hits 6715 6824 +109
- Misses 5503 5583 +80 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Some conflicts to resolve. |
|
sorry, more conflicts 😄 |
jpoehnelt
added
area: auth
area: tui
complexity: high
mkh09353
force-pushed
the
fix/setup-project-create-error-recovery
branch
from
b4de4ab to
0b53fa6
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly improves the user experience for gws auth setup by adding better error recovery for project creation and a more streamlined flow for continuing to gws auth login. The changes are well-structured, and the new error messages for gcloud failures are particularly helpful. The addition of the --login flag and the interactive prompt are also great UX enhancements. I have one suggestion to improve the robustness of the interactive wizard logic by avoiding unwrap() calls.
| let project_name = match ctx | ||
| .wizard | ||
| .as_mut() | ||
| .unwrap() | ||
| .show_input( | ||
| "Create new GCP project", | ||
| "Enter a unique project ID", | ||
| last_attempt.as_deref(), | ||
| ) | ||
| .map_err(|e| GwsError::Validation(format!("TUI error: {e}")))? | ||
| { | ||
| crate::setup_tui::InputResult::Confirmed(v) => { | ||
| let trimmed = v.trim().to_string(); | ||
| if trimmed.is_empty() { | ||
| if let Some(ref mut w) = ctx.wizard { | ||
| w.show_message("Project ID cannot be empty. Enter a valid ID, press ↑ to go back, or Esc to cancel.") | ||
| .ok(); | ||
| } | ||
| continue; | ||
| } | ||
| trimmed | ||
| } | ||
| crate::setup_tui::InputResult::GoBack => { | ||
| return Ok(SetupStage::Project); | ||
| } | ||
| crate::setup_tui::InputResult::Cancelled => { | ||
| ctx.finish_wizard(); | ||
| return Err(GwsError::Validation( | ||
| "Setup cancelled".to_string(), | ||
| )); | ||
| } | ||
| }; | ||
|
|
||
| ctx.wizard | ||
| .as_mut() | ||
| .unwrap() | ||
| .show_message(&format!("Creating project '{}'...", project_name)) | ||
| .ok(); |
There was a problem hiding this comment.
These unwrap() calls on ctx.wizard could lead to a panic if the wizard is not available, which would crash the setup process. While this code path is likely only taken in interactive mode where the wizard is present, it's safer to handle the None case explicitly.
Using ok_or_else with the ? operator would be more robust, returning a proper GwsError instead of panicking. This also improves consistency with Rust's idiomatic error handling.
let project_name = match ctx
.wizard
.as_mut()
.ok_or_else(|| GwsError::Validation("Wizard not available in interactive mode".to_string()))?
.show_input(
"Create new GCP project",
"Enter a unique project ID",
last_attempt.as_deref(),
)
.map_err(|e| GwsError::Validation(format!("TUI error: {e}")))?
{
crate::setup_tui::InputResult::Confirmed(v) => {
let trimmed = v.trim().to_string();
if trimmed.is_empty() {
if let Some(ref mut w) = ctx.wizard {
w.show_message("Project ID cannot be empty. Enter a valid ID, press ↑ to go back, or Esc to cancel.")
.ok();
}
continue;
}
trimmed
}
crate::setup_tui::InputResult::GoBack => {
return Ok(SetupStage::Project);
}
crate::setup_tui::InputResult::Cancelled => {
ctx.finish_wizard();
return Err(GwsError::Validation(
"Setup cancelled".to_string(),
));
}
};
ctx.wizard
.as_mut()
.ok_or_else(|| GwsError::Validation("Wizard not available in interactive mode".to_string()))?
.show_message(&format!("Creating project '{}'...", project_name))
.ok();3 participants
Summary
Improve
gws auth setupUX in two areas:gcloud projects createfails.FAILED_PRECONDITION/type: TOS)gws auth setup --loginto automatically continue intogws auth login.Run gws auth login now? [Y/n].Why
Users were hitting recoverable setup errors and having to restart the entire flow. Also, successful setup ending immediately felt abrupt and required an extra manual command.
Testing
cargo clippy -- -D warningscargo testBoth pass locally.