From 0466aeab53db7aad3cc7c9aab094c24af728e15c Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Fri, 24 Nov 2023 14:51:54 -0400 Subject: [PATCH 1/2] 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. --- buildpacks/nodejs-npm-install/CHANGELOG.md | 4 +++ buildpacks/nodejs-npm-install/src/main.rs | 29 ++++++++++--------- .../tests/fixtures/empty/.gitkeep | 0 .../tests/integration_test.rs | 14 +++++++++ 4 files changed, 33 insertions(+), 14 deletions(-) create mode 100644 buildpacks/nodejs-npm-install/tests/fixtures/empty/.gitkeep diff --git a/buildpacks/nodejs-npm-install/CHANGELOG.md b/buildpacks/nodejs-npm-install/CHANGELOG.md index d4f3cc405..0b7e4b863 100644 --- a/buildpacks/nodejs-npm-install/CHANGELOG.md +++ b/buildpacks/nodejs-npm-install/CHANGELOG.md @@ -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. + ## [2.3.0] - 2023-11-09 - No changes. diff --git a/buildpacks/nodejs-npm-install/src/main.rs b/buildpacks/nodejs-npm-install/src/main.rs index 149c32f8e..c90efc8de 100644 --- a/buildpacks/nodejs-npm-install/src/main.rs +++ b/buildpacks/nodejs-npm-install/src/main.rs @@ -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() } diff --git a/buildpacks/nodejs-npm-install/tests/fixtures/empty/.gitkeep b/buildpacks/nodejs-npm-install/tests/fixtures/empty/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/buildpacks/nodejs-npm-install/tests/integration_test.rs b/buildpacks/nodejs-npm-install/tests/integration_test.rs index f5ce04a8a..c7bfbde96 100644 --- a/buildpacks/nodejs-npm-install/tests/integration_test.rs +++ b/buildpacks/nodejs-npm-install/tests/integration_test.rs @@ -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(); From 80f2636d0802b2b7e4164aa41a358bcca74f621f Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Fri, 24 Nov 2023 14:53:45 -0400 Subject: [PATCH 2/2] Update CHANGELOG.md --- buildpacks/nodejs-npm-install/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildpacks/nodejs-npm-install/CHANGELOG.md b/buildpacks/nodejs-npm-install/CHANGELOG.md index 0b7e4b863..4782d0a7d 100644 --- a/buildpacks/nodejs-npm-install/CHANGELOG.md +++ b/buildpacks/nodejs-npm-install/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Detection will report a failure instead of throwing an error when there is no `package.json` file in the application directory. +- 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