Skip to content

Commit

Permalink
Raise an error at runtime when Faraday Multipart is not installed:
Browse files Browse the repository at this point in the history
- In 7bc6dd5, a warning message was
  added to let users know that faraday-multipart is required when
  using Faraday 2.0.

  This warning is not very helpful, and may lead to projects adding
  the gem to their dependencies even if they don't need it, as the
  faraday-multipart middleware is only used for uploading license for
  GitHub entrepise.

  This patch will avoid printing the warning unnecessarily but also
  display a better error message when ultimately
  the call to `conn.request :multipart` fails.

  Fix #1701
  • Loading branch information
Edouard-chin committed Sep 3, 2024
1 parent 301bb57 commit 32b73b6
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 7 deletions.
5 changes: 0 additions & 5 deletions lib/octokit/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@
rescue LoadError
Octokit::Warnable.octokit_warn 'To use retry middleware with Faraday v2.0+, install `faraday-retry` gem'
end
begin
require 'faraday/multipart'
rescue LoadError
Octokit::Warnable.octokit_warn 'To use multipart middleware with Faraday v2.0+, install `faraday-multipart` gem; note: this is used by the ManageGHES client for uploading licenses'
end
end

module Octokit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,14 @@ def password_hash
def faraday_configuration
@faraday_configuration ||= Faraday.new(url: @management_console_endpoint) do |http|
http.headers[:user_agent] = user_agent
http.request :multipart
begin
http.request :multipart
rescue Faraday::Error
raise Faraday::Error, <<~ERROR
The `faraday-multipart` gem is required to upload a license.
Please add `gem "faraday-multipart"` to your Gemfile.
ERROR
end
http.request :url_encoded

# Disabling SSL is essential for certain self-hosted Enterprise instances
Expand Down
9 changes: 8 additions & 1 deletion lib/octokit/manage_ghes_client/manage_ghes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,14 @@ def set_maintenance_mode(enabled, options = {})
# @return [nil]
def upload_license(license)
conn = authenticated_client
conn.request :multipart
begin
conn.request :multipart
rescue Faraday::Error
raise Faraday::Error, <<~ERROR
The `faraday-multipart` gem is required to upload a license.
Please add `gem "faraday-multipart"` to your Gemfile.
ERROR
end
params = {}
params[:license] = Faraday::FilePart.new(license, 'binary')
params[:password] = @manage_ghes_password
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@
expect(@enterprise_management_console_client.last_response.status).to eq(202)
assert_requested :post, github_management_console_url('setup/api/start')
end

it 'raises an error if the faraday-multipart gem is not installed' do
middleware_key = :multipart
middleware = Faraday::Request.unregister_middleware(middleware_key)

expect { @enterprise_management_console_client.upload_license(@license) }.to raise_error Faraday::Error
ensure
Faraday::Request.register_middleware(middleware_key => middleware) if middleware
end
end # .upload_license

describe '.start_configuration', :vcr do
Expand Down
9 changes: 9 additions & 0 deletions spec/octokit/ghes_manage_client/ghes_manage_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@
expect(@manage_ghes.last_response.status).to eq(202)
assert_requested :post, github_manage_ghes_url('/manage/v1/config/init')
end

it 'raises an error if the faraday-multipart gem is not installed' do
middleware_key = :multipart
middleware = Faraday::Request.unregister_middleware(middleware_key)

expect { @manage_ghes.upload_license(@license) }.to raise_error Faraday::Error
ensure
Faraday::Request.register_middleware(middleware_key => middleware) if middleware
end
end # .upload_license

describe '.start_configuration', :vcr do
Expand Down

0 comments on commit 32b73b6

Please sign in to comment.