Skip to content

feat(mobile-app): Make missing path not panic #2583

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rbro112 merged 6 commits into from
Jul 10, 2025
Merged

feat(mobile-app): Make missing path not panic #2583

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9d7585f
Make missing path not panic
rbro112 Jul 8, 2025
ce1cd6d
Update to throw error with missing path
rbro112 Jul 8, 2025
18548ae
failed
rbro112 Jul 8, 2025
f8cd83d
Make path argument required
rbro112 Jul 10, 2025
8dafa53
Error
rbro112 Jul 10, 2025
430b188
Windows
rbro112 Jul 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions src/commands/mobile_app/upload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ pub fn make_command(command: Command) -> Command {
.value_name("PATH")
.help("The path to the mobile app files to upload. Supported files include Apk, Aab or XCArchive.")
.num_args(1..)
.action(ArgAction::Append),
.action(ArgAction::Append)
.required(true),
)
.arg(
Arg::new("sha")
Expand All @@ -49,9 +50,13 @@ pub fn make_command(command: Command) -> Command {
}

pub fn execute(matches: &ArgMatches) -> Result<()> {
let path_strings = matches
.get_many::<String>("paths")
.expect("paths argument is required");
let path_strings: Vec<_> = match matches.get_many::<String>("paths") {
Some(paths) => paths.collect(),
None => {
// Clap will handle the case where no paths are provided.
return Ok(());
}
};
Comment on lines -52 to +58
Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex Jul 10, 2025

Choose a reason for hiding this comment

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

Since clap is handling this already (or at least, as it should be), I would go back to the expect (which panics) since getting into the state where we don't have paths at this point is unexpected and would indicate a bug in Sentry CLI. You can update the expect message to state that Clap is enforcing that the argument is required.


let sha = matches
.get_one("sha")
Expand Down Expand Up @@ -301,14 +306,12 @@ fn upload_file(
build_configuration.unwrap_or("unknown")
);

let chunk_upload_options = api
.get_chunk_upload_options(org)?
.ok_or_else(|| {
anyhow!(
"The Sentry server lacks chunked uploading support, which \
let chunk_upload_options = api.get_chunk_upload_options(org)?.ok_or_else(|| {
anyhow!(
"The Sentry server lacks chunked uploading support, which \
is required for mobile app uploads. {SELF_HOSTED_ERROR_HINT}"
)
})?;
)
Copy link
Copy Markdown
Member Author

@rbro112 rbro112 Jul 8, 2025

Choose a reason for hiding this comment

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

Looks like an autoformat update.

})?;

if !chunk_upload_options.supports(ChunkUploadCapability::PreprodArtifacts) {
bail!(
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/_cases/mobile_app/mobile_app-upload-help.trycmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ $ sentry-cli mobile-app upload --help
? success
[EXPERIMENTAL] Upload mobile app files to a project.

Usage: sentry-cli[EXE] mobile-app upload [OPTIONS] [PATH]...
Usage: sentry-cli mobile-app upload [OPTIONS] <PATH>...

Arguments:
[PATH]... The path to the mobile app files to upload. Supported files include Apk, Aab or
<PATH>... The path to the mobile app files to upload. Supported files include Apk, Aab or
XCArchive.

Options:
Expand Down
11 changes: 11 additions & 0 deletions tests/integration/_cases/mobile_app/mobile_app-upload-no-path.trycmd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
```
$ sentry-cli mobile-app upload
? failed
error: the following required arguments were not provided:
<PATH>...

Usage: sentry-cli mobile-app upload <PATH>...

For more information, try '--help'.

```
5 changes: 5 additions & 0 deletions tests/integration/mobile_app/upload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ fn command_mobile_app_upload_no_token() {
TestManager::new().register_trycmd_test("mobile_app/mobile_app-upload-apk-no-token.trycmd");
}

#[test]
fn command_mobile_app_upload_no_path() {
TestManager::new().register_trycmd_test("mobile_app/mobile_app-upload-no-path.trycmd");
}

#[test]
fn command_mobile_app_upload_invalid_aab() {
TestManager::new()
Expand Down
Loading