Skip to content

Commit

Permalink
Gracefully handle error when trying to read package.json in detect (#…
Browse files Browse the repository at this point in the history
…733)

* Gracefully handle error when trying to read `package.json` in detect

This change prevents the detection routine from raising an error during detect if:
- there is no `package.json`
- there is a `package.json` but it cannot be read for some reason

Both error cases will be handled and report a failed detect.

* Update CHANGELOG.md
  • Loading branch information
colincasey authored Nov 29, 2023
1 parent 8c1a52b commit f1a7028
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 14 deletions.
4 changes: 4 additions & 0 deletions buildpacks/nodejs-npm-install/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Detection will report a failure instead of throwing an error when there is no `package.json` file in the application directory. ([#733](https://github.com/heroku/buildpacks-nodejs/pull/733))

## [2.3.0] - 2023-11-09

- No changes.
Expand Down
29 changes: 15 additions & 14 deletions buildpacks/nodejs-npm-install/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,21 @@ impl Buildpack for NpmInstallBuildpack {
.try_exists()
.map_err(NpmInstallBuildpackError::Detect)?;

let package_json = PackageJson::read(context.app_dir.join("package.json"))
.map_err(NpmInstallBuildpackError::PackageJson)?;

if npm_lockfile_exists || package_json.has_dependencies() {
DetectResultBuilder::pass()
.build_plan(
BuildPlanBuilder::new()
.provides("node_modules")
.requires("npm")
.requires("node_modules")
.requires("node")
.build(),
)
.build()
if let Ok(package_json) = PackageJson::read(context.app_dir.join("package.json")) {
if npm_lockfile_exists || package_json.has_dependencies() {
DetectResultBuilder::pass()
.build_plan(
BuildPlanBuilder::new()
.provides("node_modules")
.requires("npm")
.requires("node_modules")
.requires("node")
.build(),
)
.build()
} else {
DetectResultBuilder::fail().build()
}
} else {
DetectResultBuilder::fail().build()
}
Expand Down
Empty file.
14 changes: 14 additions & 0 deletions buildpacks/nodejs-npm-install/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,20 @@ fn test_dependencies_and_missing_lockfile_errors() {
);
}

#[test]
#[ignore]
fn detect_rejects_non_npm_project() {
nodejs_integration_test_with_config(
"./fixtures/empty",
|config| {
config.expected_pack_result(PackResult::Failure);
},
|ctx| {
assert_contains!(ctx.pack_stdout, "fail: heroku/nodejs-npm-install");
},
);
}

fn add_lockfile_entry(app_dir: &Path, package_name: &str, lockfile_entry: serde_json::Value) {
update_json_file(&app_dir.join("package-lock.json"), |json| {
let dependencies = json["dependencies"].as_object_mut().unwrap();
Expand Down

0 comments on commit f1a7028

Please sign in to comment.