Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop explicitly catching missing env-var error in composer #7675

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions bin/dry-run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -441,13 +441,6 @@ def handle_dependabot_error(error:, dependency:)
"error-type": "private_source_certificate_failure",
"error-detail": { source: error.source }
}
when Dependabot::MissingEnvironmentVariable
{
"error-type": "missing_environment_variable",
"error-detail": {
"environment-variable": error.environment_variable
}
}
when Dependabot::GoModulePathMismatch
{
"error-type": "go_module_path_mismatch",
Expand Down
9 changes: 0 additions & 9 deletions common/lib/dependabot/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,6 @@ def initialize(source)
end
end

class MissingEnvironmentVariable < DependabotError
attr_reader :environment_variable

def initialize(environment_variable)
@environment_variable = environment_variable
super("Missing environment variable #{@environment_variable}")
end
end

# Useful for JS file updaters, where the registry API sometimes returns
# different results to the actual update process
class InconsistentRegistryResponse < DependabotError; end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,20 +182,6 @@ def handle_composer_errors(error)
raise Dependabot::GitDependenciesNotReachable, dependency_url
end

# NOTE: This matches an error message from composer plugins used to install ACF PRO
# https://github.com/PhilippBaschke/acf-pro-installer/blob/772cec99c6ef8bc67ba6768419014cc60d141b27/src/ACFProInstaller/Exceptions/MissingKeyException.php#L14
# https://github.com/pivvenit/acf-pro-installer/blob/f2d4812839ee2c333709b0ad4c6c134e4c25fd6d/src/Exceptions/MissingKeyException.php#L25
if error.message.start_with?("Could not find a key for ACF PRO", "Could not find a license key for ACF PRO")
raise MissingEnvironmentVariable, "ACF_PRO_KEY"
end

# NOTE: This matches error output from a composer plugin (private-composer-installer):
# https://github.com/ffraenz/private-composer-installer/blob/8655e3da4e8f99203f13ccca33b9ab953ad30a31/src/Exception/MissingEnvException.php#L22
if error.message.match?(MISSING_ENV_VAR_REGEX)
env_var = error.message.match(MISSING_ENV_VAR_REGEX).named_captures.fetch("env_var")
raise MissingEnvironmentVariable, env_var
end

if error.message.start_with?("Unknown downloader type: npm-sign") ||
error.message.include?("file could not be downloaded") ||
error.message.include?("configuration does not allow connect")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,55 +268,6 @@
end
end

context "that requires an environment variable (composer v1)" do
let(:project_name) { "v1/env_variable" }

context "that hasn't been provided" do
it "raises a MissingEnvironmentVariable error" do
expect { updated_lockfile_content }.to raise_error do |error|
expect(error).to be_a(Dependabot::MissingEnvironmentVariable)
expect(error.environment_variable).to eq("ACF_PRO_KEY")
end
end
end

context "that has been provided" do
let(:updater) do
described_class.new(
dependency_files: files,
dependencies: [dependency],
credentials: [{
"type" => "git_source",
"host" => "github.com",
"username" => "x-access-token",
"password" => "token"
}, {
"type" => "php_environment_variable",
"env-key" => "ACF_PRO_KEY",
"env-value" => "example_key"
}]
)
end

it "runs just fine (we get a 404 here because our key is wrong)" do
expect { updated_lockfile_content }.to raise_error do |error|
expect(error).to be_a(Dependabot::DependencyFileNotResolvable)
expect(error.message).to include("404")
end
end
end
end

context "that requires an environment variable (composer v2)" do
let(:project_name) { "env_variable" }

context "that hasn't been provided" do
it "does not attempt to download and has details of the updated item" do
expect(updated_lockfile_content).to include("\"version\":\"5.9.2\"")
end
end
end

context "with a path source" do
let(:project_name) { "path_source" }

Expand Down
28 changes: 0 additions & 28 deletions composer/spec/fixtures/projects/env_variable/composer.json

This file was deleted.

57 changes: 0 additions & 57 deletions composer/spec/fixtures/projects/env_variable/composer.lock

This file was deleted.

28 changes: 0 additions & 28 deletions composer/spec/fixtures/projects/v1/env_variable/composer.json

This file was deleted.

57 changes: 0 additions & 57 deletions composer/spec/fixtures/projects/v1/env_variable/composer.lock

This file was deleted.

7 changes: 0 additions & 7 deletions updater/lib/dependabot/updater/error_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,6 @@ def error_details_for(error, dependency: nil, dependency_group: nil) # rubocop:d
"error-type": "private_source_certificate_failure",
"error-detail": { source: error.source }
}
when Dependabot::MissingEnvironmentVariable
{
"error-type": "missing_environment_variable",
"error-detail": {
"environment-variable": error.environment_variable
}
}
when Dependabot::GoModulePathMismatch
{
"error-type": "go_module_path_mismatch",
Expand Down