WebMockHTTPClients#do_get returns inconsistent types

[thelowlypeon]

tl;dr WebMockHTTPClients#do_get returns HTTP::Message in some cases, and Queue::Thread in other cases. This does is fine in most cases because the return value is generally ignored when callbacks are expected to run, but breaks things in applications with other gems that modify HTTP::Client, such as ddtrace.

If the request is mocked, webmock runs this:

        webmock_response = synchronize_request_response { webmock_responses.delete(request_signature) }
        response = build_httpclient_response(webmock_response, stream, req.header, &block)
        @request_filter.each do |filter|
          filter.filter_response(req, response)
        end
        res = conn.push(response)
        WebMock::CallbackRegistry.invoke_callbacks(
          {lib: :httpclient}, request_signature, webmock_response)
        res

We can see that the return value is res, which is the return value of conn.push(response), which is a Thread::Queue.

However, if the request is not mocked (and net connect is allowed), webmock will return an HTTP::Message, as HTTP::Client does:

        ...
        res = conn.pop
        conn.push(res)
        ...
        res

It seems reasonable to assume that this isn't generally an issue because tests that stub requests will see callbacks invoked and discard the return type, and because ddtrace may not be enabled in test environments. (Though some may have it enabled to better mirror production environments and simply filter all traces.)

My understanding of HTTP::Client is limited, so I may be understanding this problem incorrectly. However, the inconsistent return type breaks seems like it may be unintentional, and breaks ddtrace when enabled in the same environment as webmock.

(fwiw, if the return value is nil, ddtrace gracefully ignores the request.)

[bblimke]

@thelowlypeon Thank you for reporting. Do I understand that the issue only occurs when ddtrace is enabled? Is this still an issue with recent versions of ddtrace?