Skip to content

Commit

Permalink
Rename paragraph::ErrorBuilder to attn::Announcement
Browse files Browse the repository at this point in the history
I've previously lamented that I didn't like that name. This change makes the name more generic and also introduces two now announcement kinds: warning and important.


In addition. The debugging details are now printed first so the most important error message comes last.
  • Loading branch information
schneems committed Aug 2, 2023
1 parent 8c2f50b commit 0de7582
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 62 deletions.
44 changes: 22 additions & 22 deletions buildpacks/ruby/src/user_errors.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
use crate::RubyBuildpackError;
use commons::{
build_output::{self, paragraph::ErrorBuilder},
build_output::{self, attn::Announcement},
fun_run::CmdError,
};
use indoc::formatdoc;

pub(crate) fn on_error(err: libcnb::Error<RubyBuildpackError>) {
match cause(err) {
Cause::OurError(error) => log_our_error(error),
Cause::FrameworkError(error) => ErrorBuilder::new()
Cause::FrameworkError(error) => Announcement::error()
.debug_details(&error)
.header(
"heroku/buildpack-ruby internal buildpack error"
).body(formatdoc! {"
Expand All @@ -25,7 +26,6 @@ pub(crate) fn on_error(err: libcnb::Error<RubyBuildpackError>) {
CLI. If you can reproduce the behavior locally and believe you've found a bug in the
buildpack or the framework please open an issue on the buildpack's GitHub repository.
"})
.debug_details(&error)
.print(),
};
}
Expand All @@ -39,7 +39,8 @@ fn log_our_error(error: RubyBuildpackError) {
match error {
RubyBuildpackError::CannotDetectRakeTasks(error) => {
let local_debug_cmd = local_command_debug(&error);
ErrorBuilder::new()
Announcement::error()
.detail(error.into())
.header(
"Error detecting rake tasks"
)
Expand All @@ -51,10 +52,10 @@ fn log_our_error(error: RubyBuildpackError) {
.body(formatdoc! {"
Use the information above to debug further.
"})
.detail(error.into())
.print();
},
RubyBuildpackError::BundleListError(error) => ErrorBuilder::new()
RubyBuildpackError::BundleListError(error) => Announcement::error()
.debug_details(&error)
.header(
"Error detecting dependencies"
)
Expand All @@ -64,22 +65,21 @@ fn log_our_error(error: RubyBuildpackError) {
Use the following information to help debug the system.
"})
.debug_details(&error)
.print(),
RubyBuildpackError::RubyInstallError(error) => ErrorBuilder::new()
RubyBuildpackError::RubyInstallError(error) => Announcement::error()
.debug_details(&error)
.header(
"Error installing Ruby",
).body(formatdoc! {"
Could not install the detected Ruby version. Ensure that you're using a supported
ruby version and try again.
"})
.url(build_output::paragraph::Url::Label {
.url(build_output::attn::Url::Label {
label: "Supported ruby versions".to_string(),
url: "https://devcenter.heroku.com/articles/ruby-support#ruby-versions".to_string(),
})
.debug_details(&error)
.print(),
RubyBuildpackError::MissingGemfileLock(error) => ErrorBuilder::new()
RubyBuildpackError::MissingGemfileLock(error) => Announcement::error()
.header(
"Gemfile.lock` not found"
)
Expand All @@ -89,23 +89,24 @@ fn log_our_error(error: RubyBuildpackError) {
If you have a `Gemfile.lock` in your application, ensure it is tracked in Git and
that you’re pushing the correct branch.
"})
.url(build_output::paragraph::Url::MoreInfo(
.url(build_output::attn::Url::MoreInfo(
"https://devcenter.heroku.com/articles/git#deploy-from-a-branch-besides-main"
.to_string(),
)).
debug_details(&error)
.print(),
RubyBuildpackError::RakeAssetsCacheError(error) => ErrorBuilder::new()
RubyBuildpackError::RakeAssetsCacheError(error) => Announcement::error()
.debug_details(&error)
.header(
"Error caching frontend assets"
)
.body(formatdoc! {"
An error occurred while attempting to cache frontend assets, and the Ruby buildpack cannot continue.
"})
.body(file_hints)
.debug_details(&error)
.print(),
RubyBuildpackError::BundleInstallDigestError(error) => ErrorBuilder::new()
RubyBuildpackError::BundleInstallDigestError(error) => Announcement::error()
.debug_details(&error)
.header(
"Failed to generate file digest"
)
Expand All @@ -119,44 +120,43 @@ fn log_our_error(error: RubyBuildpackError) {
HEROKU_SKIP_BUNDLE_DIGEST=1
"})
.debug_details(&error)
.print(),
RubyBuildpackError::BundleInstallCommandError(error) => ErrorBuilder::new()
RubyBuildpackError::BundleInstallCommandError(error) => Announcement::error()
.detail(error.into())
.header(
"Failed to install bundler"
)
.body(formatdoc! {"
The ruby package managment tool, `bundler`, failed to install. Bundler is required to install your application's dependencies listed in the `Gemfile`.
"})
.body(heroku_status)
.detail(error.into())
.print(),
RubyBuildpackError::RakeAssetsPrecompileFailed(error) => {
let cmd_debug = local_command_debug(&error);

ErrorBuilder::new()
Announcement::error()
.detail(error.into())
.header("Failed to compile assets")
.body(formatdoc! {"
An error occured while compiling assets via rake command. Details of the error are
listed below.
"})
.body(cmd_debug)
.body(git_branch)
.detail(error.into())
.print();
},
RubyBuildpackError::GemInstallBundlerCommandError(error) => {
let cmd_debug = local_command_debug(&error);

ErrorBuilder::new()
Announcement::error()
.detail(error.into())
.header("Failed to install gems")
.body(formatdoc! {"
Could not install gems to the system via bundler. Gems are dependencies
your application listed in the Gemfile and resolved in the Gemfile.lock.
"})
.body(cmd_debug)
.body(git_branch)
.detail(error.into())
.print();
},
}
Expand Down
101 changes: 61 additions & 40 deletions commons/src/build_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,16 +595,28 @@ pub mod fmt {

/// Helper method that adds a bang i.e. `!` before strings
pub(crate) fn bangify(body: impl AsRef<str>) -> String {
let body = body.as_ref();
prepend_each_line("!", " ", body)
}

// Ensures every line starts with `prepend`
pub(crate) fn prepend_each_line(
prepend: impl AsRef<str>,
separator: impl AsRef<str>,
contents: impl AsRef<str>,
) -> String {
let body = contents.as_ref();
let prepend = prepend.as_ref();
let separator = separator.as_ref();

if body.trim().is_empty() {
"!\n".to_string()
format!("{prepend}\n")
} else {
body.split('\n')
.map(|section| {
if section.trim().is_empty() {
"!".to_string()
prepend.to_string()
} else {
format!("! {section}")
format!("{prepend}{separator}{section}")
}
})
.collect::<Vec<String>>()
Expand Down Expand Up @@ -691,23 +703,17 @@ pub mod fmt {
}
}

pub mod paragraph {
pub mod attn {
use super::fmt::{ERROR_COLOR, IMPORTANT_COLOR, WARNING_COLOR};
use crate::build_output::fmt::{self, bangify, colorize};
use crate::fun_run::CmdError;
use itertools::Itertools;
use std::fmt::Display;

pub(crate) const ERROR_COLOR: &str = crate::build_output::fmt::ERROR_COLOR;

/// Holds info about a url
#[derive(Debug, Clone, Default, PartialEq)]
#[derive(Debug, Clone, PartialEq)]
pub enum Url {
#[default]
None,
Label {
label: String,
url: String,
},
Label { label: String, url: String },
MoreInfo(String),
}

Expand All @@ -722,14 +728,12 @@ pub mod paragraph {
"For more information, refer to the following documentation:\n{}",
fmt::url(url)
),
Url::None => f.write_str(""),
}
}
}

#[derive(Debug, PartialEq, Clone)]
pub enum Detail {
None,
Raw(String),
Debug(String),
Label { label: String, details: String },
Expand All @@ -738,7 +742,6 @@ pub mod paragraph {
impl Display for Detail {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Detail::None => f.write_str(""),
Detail::Raw(details) => write!(f, "{details}"),
Detail::Debug(details) => writeln!(f, "Debug information:\n\n{details}"),
Detail::Label { label, details } => writeln!(f, "{label}:\n\n{details}"),
Expand Down Expand Up @@ -825,64 +828,80 @@ pub mod paragraph {
}
}

/// Build the contents of an error for display
///
/// Designed so that additional optional fields may be added later without
/// breaking compatability.
pub struct ErrorBuilder {
/// Build the contents of an Announcement
pub struct Announcement {
name: String,
color: String,
inner: Vec<Part>,
}

impl Display for ErrorBuilder {
impl Display for Announcement {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut parts = self
.inner
.iter()
.tuple_windows::<(_, _)>()
.map(|(now, next)| {
let part = Self::format_part(now);
let part = self.format_part(now);
// If both last and next lines share a prefix then add a prefix
// to the newline separator
let sep = match (now, next) {
(Part::Detail(_), _) | (_, Part::Detail(_)) => "\n".to_string(),
_ => colorize(ERROR_COLOR, bangify("\n")),
_ => colorize(&self.color, bangify("\n")),
};

format!("{part}{sep}")
})
.collect::<Vec<_>>();

if let Some(part) = self.inner.last() {
parts.push(Self::format_part(part));
parts.push(self.format_part(part));
}

write!(f, "{}", parts.join(""))
}
}

impl ErrorBuilder {
fn format_part(part: &Part) -> String {
impl Announcement {
fn format_part(&self, part: &Part) -> String {
let part = match part {
Part::Body(body) => colorize(ERROR_COLOR, bangify(body.to_string().trim())),
Part::Url(url) => colorize(ERROR_COLOR, bangify(url.to_string().trim())),
Part::Body(body) => colorize(&self.color, bangify(body.to_string().trim())),
Part::Url(url) => colorize(&self.color, bangify(url.to_string().trim())),
Part::Detail(details) => details.to_string().trim().to_string(),
};
format!("{part}\n")
}

#[must_use]
pub fn new() -> Self {
Self { inner: Vec::new() }
pub fn error() -> Self {
Self {
name: "ERROR".to_string(),
color: ERROR_COLOR.to_string(),
inner: Vec::new(),
}
}

pub fn header(&mut self, header: impl AsRef<str>) -> &mut Self {
let header = format!("ERROR: {}", header.as_ref());
self.inner.push(Part::Body(Body::Raw(header)));
self
#[must_use]
pub fn warning() -> Self {
Self {
name: "WARNING".to_string(),
color: WARNING_COLOR.to_string(),
inner: Vec::new(),
}
}

pub fn add(&mut self, part: Part) -> &mut Self {
self.inner.push(part);
#[must_use]
pub fn important() -> Self {
Self {
name: "IMPORTANT".to_string(),
color: IMPORTANT_COLOR.to_string(),
inner: Vec::new(),
}
}

pub fn header(&mut self, header: impl AsRef<str>) -> &mut Self {
let header = format!("{}: {}", self.name, header.as_ref());
self.inner.push(Part::Body(Body::Raw(header)));
self
}

Expand All @@ -897,6 +916,8 @@ pub mod paragraph {
self
}

/// I don't love having this in here It's technically not part of the announcement
/// from a philosophical standpoint, however it makes a nice chained API
pub fn detail(&mut self, detail: Detail) -> &mut Self {
self.inner.push(Part::Detail(detail));
self
Expand All @@ -923,7 +944,7 @@ pub mod paragraph {

#[test]
fn test_error_output_with_url_and_detailsvisual() {
let actual = ErrorBuilder::new()
let actual = Announcement::error()
.header(
"Error installing Ruby"
)
Expand Down Expand Up @@ -983,7 +1004,7 @@ pub mod paragraph {
strip_control_codes(actual.clone().to_string().trim())
);

let actual = ErrorBuilder::new()
let actual = Announcement::error()
.header("Failed to compile assets")
.body("oops")
.detail(actual)
Expand Down

0 comments on commit 0de7582

Please sign in to comment.