[rubygems/rubygems] Only raise DSLError during Gemfile parsing when it's actually useful

DSLError prints the specific line in a Gemfile where the error was
raised. That's helpful when the error was explicitly raised by the
Gemfile DSL or, in the case it's implicitly raised, when the offending
code lives right in the Gemfile.

If it's an internal error, or something buried dowm in user code called
from the Gemfile, `DSLError` is not helpful since it hides the actual
culprit.

This commit tries to only raise `DSLError` in the cases mentioned above
and otherwise let the original error be raised.

https://github.com/rubygems/rubygems/commit/b30ff5a682
This commit is contained in:
David Rodríguez 2024-09-05 20:07:51 +02:00 committed by git
parent 1d72b3bd1a
commit 89eba5074e
5 changed files with 46 additions and 9 deletions

View File

@ -45,11 +45,15 @@ module Bundler
with_gemfile(gemfile) do |current_gemfile| with_gemfile(gemfile) do |current_gemfile|
contents ||= Bundler.read_file(current_gemfile) contents ||= Bundler.read_file(current_gemfile)
instance_eval(contents, current_gemfile, 1) instance_eval(contents, current_gemfile, 1)
rescue StandardError, ScriptError => e rescue GemfileEvalError => e
message = "There was an error " \ message = "There was an error evaluating `#{File.basename current_gemfile}`: #{e.message}"
"#{e.is_a?(GemfileEvalError) ? "evaluating" : "parsing"} " \ raise DSLError.new(message, current_gemfile, e.backtrace, contents)
"`#{File.basename current_gemfile}`: #{e.message}" rescue GemfileError, InvalidArgumentError, InvalidOption, DeprecatedError, ScriptError => e
message = "There was an error parsing `#{File.basename current_gemfile}`: #{e.message}"
raise DSLError.new(message, current_gemfile, e.backtrace, contents)
rescue StandardError => e
raise unless e.backtrace_locations.first.path == current_gemfile
message = "There was an error parsing `#{File.basename current_gemfile}`: #{e.message}"
raise DSLError.new(message, current_gemfile, e.backtrace, contents) raise DSLError.new(message, current_gemfile, e.backtrace, contents)
end end
end end
@ -215,7 +219,7 @@ module Bundler
end end
def github(repo, options = {}) def github(repo, options = {})
raise ArgumentError, "GitHub sources require a block" unless block_given? raise InvalidArgumentError, "GitHub sources require a block" unless block_given?
github_uri = @git_sources["github"].call(repo) github_uri = @git_sources["github"].call(repo)
git_options = normalize_hash(options).merge("uri" => github_uri) git_options = normalize_hash(options).merge("uri" => github_uri)
git_source = @sources.add_git_source(git_options) git_source = @sources.add_git_source(git_options)

View File

@ -244,4 +244,6 @@ module Bundler
status_code(39) status_code(39)
end end
class InvalidArgumentError < BundlerError; status_code(40); end
end end

View File

@ -23,7 +23,13 @@ module Bundler
# specified must match the version. # specified must match the version.
@versions = Array(versions).map do |v| @versions = Array(versions).map do |v|
op, v = Gem::Requirement.parse(normalize_version(v)) normalized_v = normalize_version(v)
unless Gem::Requirement::PATTERN.match?(normalized_v)
raise InvalidArgumentError, "#{v} is not a valid requirement on the Ruby version"
end
op, v = Gem::Requirement.parse(normalized_v)
op == "=" ? v.to_s : "#{op} #{v}" op == "=" ? v.to_s : "#{op} #{v}"
end end

View File

@ -80,7 +80,7 @@ RSpec.describe Bundler::RubyDsl do
context "with two requirements in the same string" do context "with two requirements in the same string" do
let(:ruby_version) { ">= 2.0.0, < 3.0" } let(:ruby_version) { ">= 2.0.0, < 3.0" }
it "raises an error" do it "raises an error" do
expect { subject }.to raise_error(ArgumentError) expect { subject }.to raise_error(Bundler::InvalidArgumentError)
end end
end end
@ -168,7 +168,7 @@ RSpec.describe Bundler::RubyDsl do
let(:file_content) { "ruby-#{version}@gemset\n" } let(:file_content) { "ruby-#{version}@gemset\n" }
it "raises an error" do it "raises an error" do
expect { subject }.to raise_error(Gem::Requirement::BadRequirementError, "Illformed requirement [\"#{version}@gemset\"]") expect { subject }.to raise_error(Bundler::InvalidArgumentError, "2.0.0@gemset is not a valid requirement on the Ruby version")
end end
end end

View File

@ -66,6 +66,31 @@ RSpec.describe "bundle install" do
end end
end end
context "when an internal error happens" do
let(:bundler_bug) do
create_file("bundler_bug.rb", <<~RUBY)
require "bundler"
module Bundler
class Dsl
def source(source, *args, &blk)
nil.name
end
end
end
RUBY
bundled_app("bundler_bug.rb").to_s
end
it "shows culprit file and line" do
skip "ruby-core test setup has always \"lib\" in $LOAD_PATH so `require \"bundler\"` always activates the local version rather than using RubyGems gem activation stuff, causing conflicts" if ruby_core?
install_gemfile "source 'https://gem.repo1'", requires: [bundler_bug], artifice: nil, raise_on_error: false
expect(err).to include("bundler_bug.rb:6")
end
end
context "with engine specified in symbol", :jruby_only do context "with engine specified in symbol", :jruby_only do
it "does not raise any error parsing Gemfile" do it "does not raise any error parsing Gemfile" do
install_gemfile <<-G install_gemfile <<-G