Guard against invalid middleware configuration with warning (#685)

README.md

@@ -166,6 +166,7 @@ Faraday.new(...) do |conn|
   conn.request :multipart
   conn.request :url_encoded
 
+  # Last middleware must be the adapter:
   conn.adapter :net_http
 end
 ```

lib/faraday/rack_builder.rb

@@ -84,6 +84,7 @@ def use(klass, *args, &block)
         use_symbol(Faraday::Middleware, klass, *args, &block)
       else
         raise_if_locked
+        warn_middleware_after_adapter if adapter_set?
         @handlers << self.class::Handler.new(klass, *args, &block)
       end
     end
@@ -105,6 +106,7 @@ def adapter(key, *args, &block)
     def insert(index, *args, &block)
       raise_if_locked
       index = assert_index(index)
+      warn_middleware_after_adapter if inserting_after_adapter?(index)
       handler = self.class::Handler.new(*args, &block)
       @handlers.insert(index, handler)
     end
@@ -200,6 +202,26 @@ def raise_if_locked
       raise StackLocked, "can't modify middleware stack after making a request" if locked?
     end
 
+    def warn_middleware_after_adapter
+      warn "WARNING: Unexpected middleware set after the adapter. " \
+        "This won't be supported from Faraday 1.0."
+    end
+
+    def adapter_set?
+      @handlers.any? { |handler| is_adapter?(handler) }
+    end
+
+    def inserting_after_adapter?(index)
+      adapter_index = @handlers.find_index { |handler| is_adapter?(handler) }
+      return false if adapter_index.nil?
+
+      index > adapter_index
+    end
+
+    def is_adapter?(handler)
+      handler.klass.ancestors.include? Faraday::Adapter
+    end
+
     def use_symbol(mod, key, *args, &block)
       use(mod.lookup_middleware(key), *args, &block)
     end

test/middleware_stack_test.rb

@@ -35,8 +35,9 @@ def test_allows_rebuilding
   end
 
   def test_allows_extending
-    build_stack Apple
-    @conn.use Orange
+    build_handlers_stack Apple
+    @builder.use Orange
+    @builder.adapter :test, &test_adapter
     assert_handlers %w[Apple Orange]
   end
 
@@ -46,20 +47,23 @@ def test_builder_is_passed_to_new_faraday_connection
   end
 
   def test_insert_before
-    build_stack Apple, Orange
+    build_handlers_stack Apple, Orange
     @builder.insert_before Apple, Banana
+    @builder.adapter :test, &test_adapter
     assert_handlers %w[Banana Apple Orange]
   end
 
   def test_insert_after
-    build_stack Apple, Orange
+    build_handlers_stack Apple, Orange
     @builder.insert_after Apple, Banana
+    @builder.adapter :test, &test_adapter
     assert_handlers %w[Apple Banana Orange]
   end
 
   def test_swap_handlers
-    build_stack Apple, Orange
+    build_handlers_stack Apple, Orange
     @builder.swap Apple, Banana
+    @builder.adapter :test, &test_adapter
     assert_handlers %w[Banana Orange]
   end
 
@@ -160,11 +164,21 @@ def build_stack(*handlers)
       handlers.each { |handler| b.use(*handler) }
       yield(b) if block_given?
 
-      b.adapter :test do |stub|
-        stub.get '/' do |env|
-          # echo the "X-Middleware" request header in the body
-          [200, {}, env[:request_headers]['X-Middleware'].to_s]
-        end
+      @builder.adapter :test, &test_adapter
+    end
+  end
+
+  def build_handlers_stack(*handlers)
+    @builder.build do |b|
+      handlers.each { |handler| b.use(*handler) }
+    end
+  end
+
+  def test_adapter
+    Proc.new do |stub|
+      stub.get '/' do |env|
+        # echo the "X-Middleware" request header in the body
+        [200, {}, env[:request_headers]['X-Middleware'].to_s]
       end
     end
   end
@@ -180,3 +194,67 @@ def unregister_middleware(component, key)
     component.instance_variable_get('@registered_middleware').delete(key)
   end
 end
+
+class MiddlewareStackOrderTest < Faraday::TestCase
+  def test_adding_response_middleware_after_adapter
+    response_after_adapter = lambda do
+      Faraday::RackBuilder.new do |b|
+        b.adapter :test
+        b.response :raise_error
+      end
+    end
+
+    assert_output("", expected_middleware_warning, &response_after_adapter)
+  end
+
+  def test_adding_request_middleware_after_adapter
+    request_after_adapter = lambda do
+      Faraday::RackBuilder.new do |b|
+        b.adapter :test
+        b.request :multipart
+      end
+    end
+
+    assert_output("", expected_middleware_warning, &request_after_adapter)
+  end
+
+  def test_adding_request_middleware_after_adapter_via_use
+    use_after_adapter = lambda do
+      Faraday::RackBuilder.new do |b|
+        b.adapter :test
+        b.use Faraday::Request::Multipart
+      end
+    end
+
+    assert_output("", expected_middleware_warning, &use_after_adapter)
+  end
+
+  def test_adding_request_middleware_after_adapter_via_insert
+    insert_after_adapter = lambda do
+      Faraday::RackBuilder.new do |b|
+        b.adapter :test
+        b.insert(1, Faraday::Request::Multipart)
+      end
+    end
+
+    assert_output("", expected_middleware_warning, &insert_after_adapter)
+  end
+
+  def test_adding_request_middleware_before_adapter_via_insert_no_warning
+    builder = Faraday::RackBuilder.new do |b|
+      b.adapter :test
+    end
+
+    insert_before_adapter = lambda do
+      builder.insert(0, Faraday::Request::Multipart)
+    end
+
+    assert_output("", "", &insert_before_adapter)
+  end
+
+  private
+
+  def expected_middleware_warning
+    /Unexpected middleware set after the adapter/
+  end
+end