Add explicit dependency on faraday

[olbrich]

See https://github.com/lostisland/faraday-net_http/issues/25

It is necessary to include this adapter in your gemfile after Faraday or whatever else depends on Faraday or you will get errors when trying to load your app. Declaring this dependency should fix that.

[mattbrictson]

I can confirm that if I order my Gemfile to put faraday-net_http_persistent before faraday, this leads to a crash.

gem "faraday-net_http_persistent"
gem "faraday"

gems/faraday-net_http-2.1.0/lib/faraday/net_http.rb:8:in `<module:NetHttp>': undefined method `register_middleware' for Faraday::Adapter:Class (NoMethodError)

    Faraday::Adapter.register_middleware(net_http: Faraday::Adapter::NetHttp)
                    ^^^^^^^^^^^^^^^^^^^^
  from gems/3.1.0/gems/faraday-net_http-2.1.0/lib/faraday/net_http.rb:7:in `<module:Faraday>'
  from gems/3.1.0/gems/faraday-net_http-2.1.0/lib/faraday/net_http.rb:6:in `<top (required)>'
  from gems/3.1.0/gems/faraday-net_http_persistent-2.0.1/lib/faraday/adapter/net_http_persistent.rb:3:in `require'
  from gems/3.1.0/gems/faraday-net_http_persistent-2.0.1/lib/faraday/adapter/net_http_persistent.rb:3:in `<top (required)>'
  from gems/3.1.0/gems/faraday-net_http_persistent-2.0.1/lib/faraday/net_http_persistent.rb:3:in `require_relative'
  from gems/3.1.0/gems/faraday-net_http_persistent-2.0.1/lib/faraday/net_http_persistent.rb:3:in `<top (required)>'
  from site_ruby/3.1.0/bundler/runtime.rb:73:in `require'

This seems quite fragile!

[iMacTia]

Thank you both for reporting, I'm taking a closer look. We had some circular dependencies issues, especially during the migration from Faraday v1 to v2, but we might have gotten too relaxed not 😅.

[iMacTia]

Ok, a bit of history:

• An issue was raised after the release of Faraday 2.0, citing the fact that
• The dependency was removed in this commit
• In a subsequent commit, all versions for bundled adapters in faraday were brought to ~>1.0

Based on this, I'd assume the decision of not having Faraday as a dependency was mostly in support to those running Faraday 1.9.0 or prior, since the gemspec there allowed to load v2 of these gems.

In fact, this is not a problem in other adapter gems (e.g. excon).

The only way I can think of to fix this that doesn't affect people running Faraday 1.9 or prior, is to release a new v3 of these adapter gems that adds a dependency on faraday ~> 2.0.

@olbrich @mattbrictson what do you think, would that work for you or am I missing something obvious?

[mattbrictson]

Wow, history is more complicated that I realized! Backwards compatibility is really tricky.

What if we added require "faraday" like this, like the excon adapter? Then presumably the Gemfile order wouldn't matter?

diff --git lib/faraday/net_http_persistent.rb lib/faraday/net_http_persistent.rb
index 895a3b7..a7792b8 100644
--- lib/faraday/net_http_persistent.rb
+++ lib/faraday/net_http_persistent.rb
@@ -1,5 +1,6 @@
 # frozen_string_literal: true

+require "faraday"
 require_relative "adapter/net_http_persistent"
 require_relative "net_http_persistent/version"

[iMacTia]

IIRC that breaks because Faraday 1.x requires the adapters and this causes a require loop

Wow, history is more complicated than I realized! Backwards compatibility is really tricky

It is, I suffered unspeakable pain earlier this year because of this and tried to forget 🙈, but it keeps hunting me

[mattbrictson]

Would this work?

diff --git lib/faraday/net_http_persistent.rb lib/faraday/net_http_persistent.rb
index 895a3b7..49df7ea 100644
--- lib/faraday/net_http_persistent.rb
+++ lib/faraday/net_http_persistent.rb
@@ -12,6 +12,7 @@ module Faraday
     # * conn.adapter Faraday::Adapter::NetNttpPersistent
     # * conn.adapter :net_http_persistent
     # Without this line, only the former method is valid.
+    require "faraday" unless defined?(Faraday::Adapter) && Faraday::Adapter.respond_to?(:register_middleware)
     Faraday::Adapter.register_middleware(net_http_persistent: Faraday::Adapter::NetHttpPersistent)
   end
 end

[iMacTia]

I can give it a try, though once you hit the require on that line (i.e. if faraday-net_http_persistent is loaded before faraday), then faraday will require this gem again here. If it actually tried that, thisif condition would break the loop, but I'd expect require to complain as soon as it detects it's loading a file currently being loaded.

[mattbrictson]

Here are are the scenarios:

---

gem "faraday", "~> 1.0"
gem "faraday-net_http_persistent"

✅ OK

---

gem "faraday-net_http_persistent"
gem "faraday", "~> 1.0"

❌ gems/faraday-net_http-2.1.0/lib/faraday/net_http.rb:8:in <module:NetHttp>': undefined methodregister_middleware' for Faraday::Adapter:Class (NoMethodError)

---

gem "faraday", "~> 2.0"
gem "faraday-net_http_persistent"

✅ OK

---

gem "faraday-net_http_persistent"
gem "faraday", "~> 2.0"

❌ gems/faraday-net_http-2.1.0/lib/faraday/net_http.rb:8:in <module:NetHttp>': undefined methodregister_middleware' for Faraday::Adapter:Class (NoMethodError)

---

If I apply the following patch:

diff --git lib/faraday/net_http_persistent.rb lib/faraday/net_http_persistent.rb
index 895a3b7..c609176 100644
--- lib/faraday/net_http_persistent.rb
+++ lib/faraday/net_http_persistent.rb
@@ -1,5 +1,6 @@
 # frozen_string_literal: true

+require "faraday" unless defined?(Faraday::Adapter) && Faraday::Adapter.respond_to?(:register_middleware)
 require_relative "adapter/net_http_persistent"
 require_relative "net_http_persistent/version"

Then these all work for me:

---

gem "faraday", "~> 1.0"
gem "faraday-net_http_persistent", path: "../Code/lostisland/faraday-net_http_persistent/"

✅ OK

---

gem "faraday-net_http_persistent", path: "../Code/lostisland/faraday-net_http_persistent/"
gem "faraday", "~> 1.0"

✅ OK

---

gem "faraday", "~> 2.0"
gem "faraday-net_http_persistent", path: "../Code/lostisland/faraday-net_http_persistent/"

✅ OK

---

gem "faraday-net_http_persistent", path: "../Code/lostisland/faraday-net_http_persistent/"
gem "faraday", "~> 2.0"

✅ OK

[iMacTia]

I was going crazy because this was working for me as well and I couldn't understand why. Until I realised this doesn't really raises an error, but a warning, and recent versions of ruby have disabled warnings by default. Here is a one-file ruby script to reproduce:

#!/usr/bin/env ruby
# frozen_string_literal: true

$VERBOSE = true # enable warnings 

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'

  gem "faraday-net_http_persistent", path: "../../lostisland/faraday-net_http_persistent/"
  gem "faraday", '~> 1.9.0' # this issue is only with 1.9.0 or prior, as 1.9.1 fixes adapter gems versions to ~> 1.0
end

require 'irb'
IRB.start(__FILE__)

[iMacTia]

Though at this point, if we're just talking about a warning, then maybe a major release is not worth it? A warning is annoying, agreed, but upgrading Faraday to 1.9.1 would fix it anyway 🤔. I'll give this some more thoughts, but maybe at this point we could just release a new version 2.0.2 which is only compatible with Faraday 2.0 and thus won't be pulled in older projects

[iMacTia]

@mattbrictson @olbrich this PR should fix this, any chance you could give it a try and let me know if it works for your specific bundle? Remember you can remove faraday from it now as faraday-net_http_persistent will pull that in for you 👍