Potential issue with the enable/disable functionality in WebMock integration

[remnantkevin]

Introduction

I'm not sure if this is a bug, or if maybe I'm misunderstanding/misusing the httpx integration with WebMock, but I thought I would bring it up here and hopefully get clarification either way.

Description

When using the httpx adapter for WebMock, I've run into a problem where the original/real HTTPX::Session eventually gets 'lost', and the mocked version sticks around even after calling WebMock.disable!. I've tried to distil the problem down to the specific logic that I think is causing the issue. See below for more details.

Reproduction

An online executable copy of this reproduction is available at replit.com.

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'
  
  gem 'httpx'
  gem 'minitest'
  gem 'webmock'
end

require "minitest/autorun"
require "webmock/minitest"
require "httpx/adapters/webmock"

class TestEnable < Minitest::Test
  def setup
    WebMock.enable!  # A
    WebMock.disable! # B
    stub_http_request(:get, "https://www.google.com").and_return(status: 200, body: "here's google")
  end

  def test_httpx_call
    response = HTTPX.get("https://www.google.com")
    assert response.status == 200
    assert response.body.to_s == "here's google"  # C
  end
end

Details

At one level, the issue comes down to calling WebMock.enable! (A) and then WebMock.disable! (B). In the reproduction, I would expect the assertion at C to fail, given that WebMock has been disabled (B). However, it doesn't fail. But, if I remove line A (so no enable, just a disable), then the assertion at C does fail.

However, if we look at the enable! and disable! adapter code, it's not immediately clear why this would be the case:

# httpx/adapters/webmock.rb

module WebMock
  module HttpLibAdapters

    # ...snipped...

    class HttpxAdapter < HttpLibAdapter
      adapter_for :httpx

      class << self
        def enable!
          @original_session = HTTPX::Session

          webmock_session = HTTPX.plugin(Plugin)

          HTTPX.send(:remove_const, :Session)
          HTTPX.send(:const_set, :Session, webmock_session.class)
        end

        def disable!
          return unless @original_session

          HTTPX.send(:remove_const, :Session)
          HTTPX.send(:const_set, :Session, @original_session)
        end
      end
    end
  end
end

But looking at the WebMock enable! and disable! code, which calls the httpx adapter code, reveals a bit more:

# webmock/webmock.rb

module WebMock

  # ...snipped...

  def self.disable!(options = {})
    except = [options[:except]].flatten.compact
    HttpLibAdapterRegistry.instance.each_adapter do |name, adapter|
      adapter.enable!
      adapter.disable! unless except.include?(name)
    end
  end

  def self.enable!(options = {})
    except = [options[:except]].flatten.compact
    HttpLibAdapterRegistry.instance.each_adapter do |name, adapter|
      adapter.disable!
      adapter.enable! unless except.include?(name)
    end
  end

  # ...snipped...

end

What the above WebMock code shows is that whenever WebMock.enable! is called, all adapters are first disabled and then enabled. And vice versa whenever WebMock.disable! is called.

Seeing this made the problem a little clearer. The following steps happen in the reproduction code above:

1. WebMock.enable! (A)
   1. HttpxAdapter.disable!
      • returns because @original_session is nil
   2. HttpxAdapter.enable!
      • stores current (real) HTTPX::Session in @original_session
      • sets HTTPX::Session to be the mocked session
2. WebMock.disable! (B)
   1. HttpxAdapter.enable!
      • stores current (mocked) HTTPX::Session in @original_session (<-- this is where we lose a pointer to the real HTTPX::Session)
      • sets HTTPX::Session to be the mocked session
   2. HttpxAdapter.disable!
      • sets HTTPX::Session to be the @original_session, which by this stage is the mocked session, not the real session

So, in some way the actual problem is that WebMock (unexpectedly, to me at least) enables before disabling and disables before enabling. Given that WebMock does this, the httpx adapter can account for it in a variety of ways. One of which could be to make sure @original_session only ever stores the real HTTPX::Session:

# httpx/adapters/webmock.rb

class << self
  def enable!
    # Only set `@original_session` the first time, when `HTTPX::Session` will be the real session.
    # So `@original_session` will then only ever be `nil` or hold a pointer to the real `HTTPX::Session`.
    @original_session = HTTPX::Session unless @original_session 

    webmock_session = HTTPX.plugin(Plugin)

    HTTPX.send(:remove_const, :Session)
    HTTPX.send(:const_set, :Session, webmock_session.class)
  end

  def disable!
    return unless @original_session

    HTTPX.send(:remove_const, :Session)
    HTTPX.send(:const_set, :Session, @original_session)
  end
end

I'm sure there are a bunch of ways to solve the problem: e.g. another way may be to early return enable! if @original_session has been set (which indicates that it is already enabled); and early return disable! if @original_session is nil (which indicates that it is already disabled), and set @original_session to nil if disable! is not early returned. But I haven't thought this through fully.

These are just some suggestions. I'm sure you'll have a better idea of the best way to handle this.

I also may have totally misunderstood something here. Apologies if that is the case 🙂

[HoneyryderChuck]

@remnantkevin rhx for the thorough investigation, wasn't aware of this quirk.

Did you try the patch locally, and confirmed it works? If so, I'd be happy to accept your patch, feel free to open a merge request.

[remnantkevin]

I have tried the patch locally and have tested it works with the reproduction code shown above. I have not done any further testing / more thorough testing. I will open a MR on GitLab and we can take it from there.

[remnantkevin]

Merge request on GitLab: https://gitlab.com/os85/httpx/-/merge_requests/247