Skip to content

Commit 8427798

Browse files
schneemsschneems
committed
Refactor errors to use CmdError instead of status code
The previous code reported the status but not always the output (when commands are hidden).
1 parent 850e500 commit 8427798

File tree

11 files changed

+60
-127
lines changed

11 files changed

+60
-127
lines changed

‎Cargo.lock

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎buildpacks/gradle/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ libherokubuildpack = { version = "=0.26.0", default-features = false, features =
1414
nom = "7"
1515
serde = { version = "1", features = ["derive"] }
1616
tempfile = "3.15.0"
17+
fun_run = "0.4.0"
1718

1819
[dev-dependencies]
1920
buildpacks-jvm-shared-test.workspace = true

‎buildpacks/gradle/src/errors.rs

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
1-
use std::process::Output;
2-
31
use crate::GradleBuildpackError;
42
use buildpacks_jvm_shared::{
5-
log::{
6-
log_build_tool_io_error, log_build_tool_unexpected_exit_code_error,
7-
log_please_try_again_error,
8-
},
9-
output::{print_error, CmdError},
3+
log::{log_build_tool_command_error, log_please_try_again_error},
4+
output::print_error,
105
};
116
use indoc::indoc;
127

@@ -28,16 +23,9 @@ pub(crate) fn on_error_gradle_buildpack(error: GradleBuildpackError) {
2823
"},
2924
);
3025
}
31-
GradleBuildpackError::GradleBuildFailedCommand(error) => match error {
32-
CmdError::SystemError(_, error) => log_build_tool_io_error("Gradle", error),
33-
CmdError::NonZeroExitNotStreamed(named_output)
34-
| CmdError::NonZeroExitAlreadyStreamed(named_output) => {
35-
log_build_tool_unexpected_exit_code_error(
36-
"Gradle",
37-
Into::<Output>::into(named_output.clone()).status,
38-
);
39-
}
40-
},
26+
GradleBuildpackError::GradleBuildFailedCommand(error) => {
27+
log_build_tool_command_error("Gradle", &error);
28+
}
4129
GradleBuildpackError::GetTasksError(error) => log_please_try_again_error(
4230
"Failed to get Gradle tasks",
4331
"Failed to get Gradle tasks",

‎buildpacks/gradle/src/gradle_command/daemon.rs

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::gradle_command::GradleCommandError;
22
use crate::GRADLE_TASK_NAME_HEROKU_START_DAEMON;
3-
use buildpacks_jvm_shared::output::{self, CmdError};
3+
use buildpacks_jvm_shared::output;
44
use libcnb::Env;
55
use std::path::{Path, PathBuf};
66
use std::process::{Command, Stdio};
@@ -15,16 +15,16 @@ pub(crate) struct GradleDaemon {
1515
impl GradleDaemon {
1616
pub(crate) fn stop(self, gradle_env: &Env) -> Result<(), GradleCommandError<()>> {
1717
let file = self.file.reopen().map_err(GradleCommandError::Io)?;
18-
let _ = Command::new(self.executable_path)
18+
let mut command = Command::new(self.executable_path);
19+
command
1920
.args(["-q", "--stop"])
2021
.envs(gradle_env)
2122
.stdout(Stdio::from(
2223
file.try_clone().map_err(GradleCommandError::Io)?,
2324
))
24-
.stderr(Stdio::from(file))
25-
.spawn()
26-
.and_then(|mut child| child.wait())
27-
.map_err(GradleCommandError::Io)?;
25+
.stderr(Stdio::from(file));
26+
27+
output::run_command(command, true).map_err(GradleCommandError::FailedCommand)?;
2828

2929
Ok(())
3030
}
@@ -55,17 +55,7 @@ pub(crate) fn start(
5555
));
5656
command.stderr(Stdio::from(file));
5757

58-
let _ = output::run_command(command, true).map_err(|error| match error {
59-
CmdError::SystemError(_, error) => GradleCommandError::Io(error),
60-
CmdError::NonZeroExitNotStreamed(named_output)
61-
| CmdError::NonZeroExitAlreadyStreamed(named_output) => {
62-
GradleCommandError::UnexpectedExitStatus {
63-
status: *named_output.status(),
64-
stdout: named_output.stdout_lossy(),
65-
stderr: named_output.stderr_lossy(),
66-
}
67-
}
68-
})?;
58+
let _ = output::run_command(command, true).map_err(GradleCommandError::FailedCommand)?;
6959

7060
Ok(daemon)
7161
}

‎buildpacks/gradle/src/gradle_command/dependency_report.rs

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::gradle_command::GradleCommandError;
2+
use buildpacks_jvm_shared::output::run_command;
23
use libcnb::Env;
34
use std::collections::BTreeMap;
45
use std::path::Path;
@@ -8,27 +9,19 @@ pub(crate) fn dependency_report(
89
app_dir: &Path,
910
env: &Env,
1011
) -> Result<GradleDependencyReport, GradleCommandError<()>> {
11-
let output = Command::new(app_dir.join("gradlew"))
12+
let mut command = Command::new(app_dir.join("gradlew"));
13+
command
1214
.current_dir(app_dir)
1315
.envs(env)
14-
.args(["--quiet", "dependencies"])
15-
.output()
16-
.map_err(GradleCommandError::Io)?;
16+
.args(["--quiet", "dependencies"]);
1717

18-
let stdout = String::from_utf8_lossy(&output.stdout);
19-
let stderr = String::from_utf8_lossy(&output.stderr);
20-
21-
if output.status.success() {
22-
parser::dependency_report(&stdout)
23-
.map_err(|_| GradleCommandError::Parse(()))
24-
.map(|(_, dependency_report)| dependency_report)
25-
} else {
26-
Err(GradleCommandError::UnexpectedExitStatus {
27-
status: output.status,
28-
stdout: stdout.into_owned(),
29-
stderr: stderr.into_owned(),
18+
run_command(command, true)
19+
.map_err(GradleCommandError::FailedCommand)
20+
.and_then(|output| {
21+
parser::dependency_report(String::from_utf8_lossy(&output.stdout).as_ref())
22+
.map_err(|_| GradleCommandError::Parse(()))
23+
.map(|(_, dependency_report)| dependency_report)
3024
})
31-
}
3225
}
3326

3427
#[derive(Debug, Clone, Eq, PartialEq)]

‎buildpacks/gradle/src/gradle_command/mod.rs

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,18 @@ mod daemon;
22
mod dependency_report;
33
mod tasks;
44

5+
use buildpacks_jvm_shared::output::CmdError;
56
pub(crate) use daemon::start as start_daemon;
67
pub(crate) use dependency_report::{dependency_report, GradleDependencyReport};
8+
use fun_run::CommandWithName;
79
pub(crate) use tasks::tasks;
810

911
use std::process::Command;
1012

1113
#[derive(Debug)]
1214
pub(crate) enum GradleCommandError<P> {
1315
Io(std::io::Error),
14-
UnexpectedExitStatus {
15-
status: std::process::ExitStatus,
16-
stdout: String,
17-
stderr: String,
18-
},
16+
FailedCommand(CmdError),
1917
Parse(P),
2018
}
2119

@@ -25,17 +23,9 @@ impl<P> GradleCommandError<P> {
2523
F: Fn(P) -> T,
2624
{
2725
match self {
26+
GradleCommandError::FailedCommand(e) => GradleCommandError::FailedCommand(e),
2827
GradleCommandError::Parse(p) => GradleCommandError::Parse(f(p)),
2928
GradleCommandError::Io(io_error) => GradleCommandError::Io(io_error),
30-
GradleCommandError::UnexpectedExitStatus {
31-
status,
32-
stdout,
33-
stderr,
34-
} => GradleCommandError::UnexpectedExitStatus {
35-
status,
36-
stdout,
37-
stderr,
38-
},
3929
}
4030
}
4131
}
@@ -44,18 +34,9 @@ fn run_gradle_command<T, F, P>(command: &mut Command, parser: F) -> Result<T, Gr
4434
where
4535
F: FnOnce(&str, &str) -> Result<T, P>,
4636
{
47-
let output = command.output().map_err(GradleCommandError::Io)?;
37+
let output = command
38+
.named_output()
39+
.map_err(GradleCommandError::FailedCommand)?;
4840

49-
let stdout = String::from_utf8_lossy(&output.stdout).into_owned();
50-
let stderr = String::from_utf8_lossy(&output.stderr).into_owned();
51-
52-
if output.status.success() {
53-
parser(&stdout, &stderr).map_err(GradleCommandError::Parse)
54-
} else {
55-
Err(GradleCommandError::UnexpectedExitStatus {
56-
status: output.status,
57-
stdout,
58-
stderr,
59-
})
60-
}
41+
parser(&output.stdout_lossy(), &output.stderr_lossy()).map_err(GradleCommandError::Parse)
6142
}

‎buildpacks/maven/src/errors.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
use crate::{MavenBuildpackError, SettingsError};
22
use buildpacks_jvm_shared::log::{
3-
log_build_tool_io_error, log_build_tool_unexpected_exit_code_error, log_please_try_again,
4-
log_please_try_again_error,
3+
log_build_tool_command_error, log_please_try_again, log_please_try_again_error,
54
};
6-
use buildpacks_jvm_shared::output::{print_error, CmdError};
5+
use buildpacks_jvm_shared::output::print_error;
76
use buildpacks_jvm_shared::system_properties::ReadSystemPropertiesError;
87
use indoc::formatdoc;
98

@@ -82,13 +81,7 @@ pub(crate) fn on_error_maven_buildpack(error: MavenBuildpackError) {
8281
"},
8382
error
8483
),
85-
MavenBuildpackError::MavenFailedCommand(error) => {
86-
match error {
87-
CmdError::SystemError(_, error) => log_build_tool_io_error("Maven", error),
88-
CmdError::NonZeroExitNotStreamed(named_output) |
89-
CmdError::NonZeroExitAlreadyStreamed(named_output) => log_build_tool_unexpected_exit_code_error("Maven",*named_output.status()),
90-
}
91-
}
84+
MavenBuildpackError::MavenFailedCommand(error) => log_build_tool_command_error("Maven", &error),
9285
MavenBuildpackError::CannotSplitMavenCustomOpts(error) => print_error(
9386
"Invalid MAVEN_CUSTOM_OPTS",
9487
formatdoc! {"

‎buildpacks/sbt/src/errors.rs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::layers::sbt_extras::SbtExtrasLayerError;
33
use crate::layers::sbt_global::SbtGlobalLayerError;
44
use crate::sbt::output::SbtError;
55
use crate::sbt::version::ReadSbtVersionError;
6-
use buildpacks_jvm_shared::log::log_please_try_again_error;
6+
use buildpacks_jvm_shared::log::{log_build_tool_command_error, log_please_try_again_error};
77
use buildpacks_jvm_shared::output::{print_error, CmdError};
88
use buildpacks_jvm_shared::system_properties::ReadSystemPropertiesError;
99
use indoc::formatdoc;
@@ -20,7 +20,7 @@ pub(crate) enum SbtBuildpackError {
2020
UnsupportedSbtVersion(Version),
2121
DetectPhaseIoError(std::io::Error),
2222
FailedCommand(CmdError),
23-
MissingTaskFailedCommand(SbtError, CmdError),
23+
MissingTaskFailedCommand(Option<SbtError>, CmdError),
2424
ReadSbtBuildpackConfigurationError(ReadSbtBuildpackConfigurationError),
2525
ReadSystemPropertiesError(ReadSystemPropertiesError),
2626
}
@@ -140,15 +140,7 @@ pub(crate) fn log_user_errors(error: SbtBuildpackError) {
140140
}
141141
}
142142
}
143-
144-
SbtBuildpackError::FailedCommand(error) => log_please_try_again_error(
145-
"Running sbt failed",
146-
formatdoc! { "
147-
An unexpected IO error occurred while running sbt.
148-
"},
149-
error,
150-
),
151-
SbtBuildpackError::MissingTaskFailedCommand(SbtError::MissingTask(task_name), _error) => log_error(
143+
SbtBuildpackError::MissingTaskFailedCommand(Some(SbtError::MissingTask(task_name)), _error) => log_error(
152144
"Failed to run sbt!",
153145
formatdoc! {"
154146
It looks like your build.sbt does not have a valid '{task_name}' task. Please reference our Dev Center article for
@@ -157,6 +149,8 @@ pub(crate) fn log_user_errors(error: SbtBuildpackError) {
157149
https://devcenter.heroku.com/articles/scala-support#build-behavior
158150
"},
159151
),
152+
SbtBuildpackError::FailedCommand(error) |
153+
SbtBuildpackError::MissingTaskFailedCommand(_, error) => log_build_tool_command_error("Sbt", &error),
160154
SbtBuildpackError::DetectPhaseIoError(error) => log_please_try_again_error(
161155
"Unexpected I/O error",
162156
"An unexpected error occurred during the detect phase.",

‎buildpacks/sbt/src/main.rs

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use libcnb::generic::{GenericMetadata, GenericPlatform};
2121
use libcnb::{buildpack_main, Buildpack, Env, Error, Platform};
2222
use libherokubuildpack::error::on_error as on_buildpack_error;
2323
use sbt::output::parse_errors;
24-
use std::process::{Command, Output};
24+
use std::process::Command;
2525
use std::time::Instant;
2626

2727
use buildpacks_jvm_shared::output;
@@ -115,17 +115,14 @@ impl Buildpack for SbtBuildpack {
115115

116116
output::run_command(command, false)
117117
}
118-
.map_err(|error| {
119-
if let Some(missing_task) = match &error {
120-
output::CmdError::SystemError(_, _) => None,
121-
output::CmdError::NonZeroExitNotStreamed(named_output)
122-
| output::CmdError::NonZeroExitAlreadyStreamed(named_output) => {
123-
parse_errors(&Into::<Output>::into(named_output.clone()).stdout)
124-
}
125-
} {
126-
SbtBuildpackError::MissingTaskFailedCommand(missing_task, error)
127-
} else {
128-
SbtBuildpackError::FailedCommand(error)
118+
.map_err(|error| match &error {
119+
output::CmdError::SystemError(_, _) => SbtBuildpackError::FailedCommand(error),
120+
output::CmdError::NonZeroExitNotStreamed(named_output)
121+
| output::CmdError::NonZeroExitAlreadyStreamed(named_output) => {
122+
SbtBuildpackError::MissingTaskFailedCommand(
123+
parse_errors(named_output.stdout()),
124+
error,
125+
)
129126
}
130127
})?;
131128

‎shared/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@ workspace = true
1010
indoc = "2"
1111
java-properties = "2"
1212
bullet_stream = "0.4.0"
13-
fun_run = { version = "0.2.0" }
13+
fun_run = { version = "0.4.0" }

0 commit comments

Comments
 (0)