Skip to content

Commit

Permalink
Report io error when detect cannot read a file.
Browse files Browse the repository at this point in the history
It's best practice to use `try_exists()` instead of `exists()` when checking if a file exists or not because if there's a problem with the file (such as a permissions issue) then the check might fail even though the file is on disk. This seems like not a big deal, but can be very confusing when telling users "we didn't do X because you are missing file Y" but when they debug they see file Y and don't realize that the real problem isn't "missing file" but it's the hidden error preventing `exists()` from returning a true.


This commit uses fs_err_try_exists() from fs-err library that was introduced in: andrewhickman/fs-err#48.

There are a few other places where `exists` is still called, those are nested in areas that don't already return a Result so they're harder to swap.
  • Loading branch information
schneems committed Nov 28, 2023
1 parent 6a8896e commit ec7f3f8
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 9 deletions.
5 changes: 5 additions & 0 deletions buildpacks/ruby/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- The `fun_run` commons library was moved to it's own crate ([#232](https://github.com/heroku/buildpacks-ruby/pull/232))

### Added

- Raise a helpful error when a file cannot be accessed at the time of buildpack detection ([#243](https://github.com/heroku/buildpacks-ruby/pull/243))


## [2.1.2] - 2023-10-31

### Fixed
Expand Down
61 changes: 53 additions & 8 deletions buildpacks/ruby/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use commons::output::warn_later::WarnGuard;
#[allow(clippy::wildcard_imports)]
use commons::output::{build_log::*, fmt};
use core::str::FromStr;
use fs_err::PathExt;
use fun_run::CmdError;
use layers::{
bundle_download_layer::{BundleDownloadLayer, BundleDownloadLayerMetadata},
Expand Down Expand Up @@ -37,6 +38,21 @@ use clap as _;

struct RubyBuildpack;

#[derive(Debug, thiserror::Error)]
enum DetectError {
#[error("Cannot read Gemfile {0}")]
Gemfile(std::io::Error),

#[error("Cannot read Gemfile.lock {0}")]
GemfileLock(std::io::Error),

#[error("Cannot read package.json {0}")]
PackageJson(std::io::Error),

#[error("Cannot read yarn.lock {0}")]
YarnLock(std::io::Error),
}

impl Buildpack for RubyBuildpack {
type Platform = GenericPlatform;
type Metadata = GenericMetadata;
Expand All @@ -45,21 +61,49 @@ impl Buildpack for RubyBuildpack {
fn detect(&self, context: DetectContext<Self>) -> libcnb::Result<DetectResult, Self::Error> {
let mut plan_builder = BuildPlanBuilder::new().provides("ruby");

if let Ok(lockfile) = fs_err::read_to_string(context.app_dir.join("Gemfile.lock")) {
let lockfile = context.app_dir.join("Gemfile.lock");

if lockfile
.fs_err_try_exists()
.map_err(DetectError::GemfileLock)
.map_err(RubyBuildpackError::BuildpackDetectionError)?
{
plan_builder = plan_builder.requires("ruby");

if context.app_dir.join("package.json").exists() {
if context
.app_dir
.join("package.json")
.fs_err_try_exists()
.map_err(DetectError::PackageJson)
.map_err(RubyBuildpackError::BuildpackDetectionError)?
{
plan_builder = plan_builder.requires("node");
}

if context.app_dir.join("yarn.lock").exists() {
if context
.app_dir
.join("yarn.lock")
.fs_err_try_exists()
.map_err(DetectError::YarnLock)
.map_err(RubyBuildpackError::BuildpackDetectionError)?
{
plan_builder = plan_builder.requires("yarn");
}

if needs_java(&lockfile) {
if fs_err::read_to_string(lockfile)
.map_err(DetectError::GemfileLock)
.map_err(RubyBuildpackError::BuildpackDetectionError)
.map(needs_java)?
{
plan_builder = plan_builder.requires("jdk");
}
} else if context.app_dir.join("Gemfile").exists() {
} else if context
.app_dir
.join("Gemfile")
.fs_err_try_exists()
.map_err(DetectError::Gemfile)
.map_err(RubyBuildpackError::BuildpackDetectionError)?
{
plan_builder = plan_builder.requires("ruby");
}

Expand Down Expand Up @@ -232,13 +276,14 @@ impl Buildpack for RubyBuildpack {
}
}

fn needs_java(gemfile_lock: &str) -> bool {
fn needs_java(gemfile_lock: impl AsRef<str>) -> bool {
let java_regex = regex::Regex::new(r"\(jruby ").expect("clippy");
java_regex.is_match(gemfile_lock)
java_regex.is_match(gemfile_lock.as_ref())
}

#[derive(Debug)]
enum RubyBuildpackError {
pub(crate) enum RubyBuildpackError {
BuildpackDetectionError(DetectError),
RakeDetectError(CmdError),
GemListGetError(CmdError),
RubyInstallError(RubyInstallError),
Expand Down
58 changes: 57 additions & 1 deletion buildpacks/ruby/src/user_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use commons::output::{
fmt::{self, DEBUG_INFO},
};

use crate::RubyBuildpackError;
use crate::{DetectError, RubyBuildpackError};
use fun_run::{CmdError, CommandWithName};
use indoc::formatdoc;

Expand Down Expand Up @@ -45,6 +45,62 @@ fn log_our_error(mut log: Box<dyn StartedLogger>, error: RubyBuildpackError) {
let rubygems_status_url = fmt::url("https://status.rubygems.org/");

match error {
RubyBuildpackError::BuildpackDetectionError(DetectError::Gemfile(error)) => {
log.announce().error(&formatdoc! {"
Error: `Gemfile` found with error
There was an error trying to read the contents of the application's Gemfile.
The buildpack cannot continue if the Gemfile is unreadable.
{error}
Debug using the above information and try again.
"});
}
RubyBuildpackError::BuildpackDetectionError(DetectError::PackageJson(error)) => {
log.announce().error(&formatdoc! {"
Error: `package.json` found with error
The Ruby buildpack detected a package.json file but it is not readable
due to the following errors:
{error}
If your application does not need any node dependencies installed you
may delete this file and try again.
If you are expecting node dependencies to be installed, please
debug using the above information and try again.
"});
}
RubyBuildpackError::BuildpackDetectionError(DetectError::GemfileLock(error)) => {
log.announce().error(&formatdoc! {"
Error: `Gemfile.lock` found with error
There was an error trying to read the contents of the application's Gemfile.lock.
The buildpack cannot continue if the Gemfile is unreadable.
{error}
Debug using the above information and try again.
"});
}
RubyBuildpackError::BuildpackDetectionError(DetectError::YarnLock(error)) => {
log.announce().error(&formatdoc! {"
Error: `yarn.lock` found with error
The Ruby buildpack detected a yarn.lock file but it is not readable
due to the following errors:
{error}
If your application does not need any yarn installed you
may delete this file and try again.
If you are expecting yarn to be installed, please
debug using the above information and try again.
"});
}
RubyBuildpackError::MissingGemfileLock(path, error) => {
log = log
.section(&format!(
Expand Down

0 comments on commit ec7f3f8

Please sign in to comment.