PATCH method doesn't send body

[iMacTia]

Hello, while updating our tests in Faraday, I noticed a strange behaviour from Patron. It seems like when providing a body (data) to a patch call, the body gets discarded. The some doesn't happen, however, for other methods including put. Below a script to reproduce the issue. I suppose this is a bug, but if that's not the case and this is instead intended behaviour then please let me know and I'll update tests expectation.

Patron version: 0.10.0

Script to reproduce:

require 'patron'
require 'webmock'

include WebMock::API

WebMock.enable!

stub_request(:patch, "http://example.com/")
  .with(body: {"name"=>"zack"})

session = Patron::Session.new
session.patch('http://example.com', "name=zack")
# => Raises WebMock::NetConnectNotAllowedError

The same doesn't happen, however, if we use :put instead of :patch.

require 'patron'
require 'webmock'

include WebMock::API

WebMock.enable!

stub_request(:put, "http://example.com/")
  .with(body: {"name"=>"zack"})

session = Patron::Session.new
session.put('http://example.com', "name=zack")
# No exception raised

[julik]

Thanks for reporting! This doesn't match what I am seeing. If I run the following server as a test:

rackup -b 'run ->(env) { $stderr.puts env["rack.input"].read; [200, {}, []] }'

and do a PATCH directed at it with a given request body:

julik@jet patron (patch-body-check) $ brake shell
install -c tmp/x86_64-darwin16/session_ext/2.3.3/session_ext.bundle lib/patron/session_ext.bundle
cp tmp/x86_64-darwin16/session_ext/2.3.3/session_ext.bundle tmp/x86_64-darwin16/stage/lib/patron/session_ext.bundle
irb -I./lib -I./ext -r patron
irb(main):001:0> s = Patron::Session.new
=> #<Patron::Session:0x007fd5c106bc30 @headers={"User-Agent"=>"Patron/Ruby-0.12.1-libcurl/7.54.0 LibreSSL/2.0.20 zlib/1.2.11 nghttp2/1.24.0"}, @timeout=5, @connect_timeout=1, @max_redirects=5, @auth_type=:basic, @force_ipv4=false>
irb(main):003:0> s.patch('http://localhost:9292/foo', 'Ohai there')
=> #<Patron::Response @status_line='HTTP/1.1 200 OK'>

then I see the following in the terminal running the rackup server

Puma starting in single mode...
* Version 3.11.3 (ruby 2.3.3-p222), codename: Love Song
* Min threads: 5, max threads: 5
* Environment: development
* Listening on tcp://localhost:9292
Use Ctrl-C to stop
Ohai there
::1 - - [15/Mar/2018:00:55:03 +0100] "PATCH /foo HTTP/1.1" 200 - 0.0010

If I try to pass in the body: keyword argument it seems that it gets converted to the form encoded request body, as designed:

irb(main):004:0> s.patch('http://localhost:9292/foo', body: 'Ohai there')
=> #<Patron::Response @status_line='HTTP/1.1 200 OK'>

I see in the rackup terminal

body=Ohai there
::1 - - [15/Mar/2018:00:59:14 +0100] "PATCH /foo HTTP/1.1" 200 - 0.0007

This means that the request body does get passed to rack.input correctly and does, indeed, get sent to the server. Due to the recent experience in #159 I tend to suspect that this is a Wembock regression - Webmock is extremely aggressive in the way it monkeypatches Patron, and that way hardly matches the design of the library anymore. Even considering all the steps I took to keep the API compatible.

So far I remember giving a recommendation to switch Webmock from Patron to Faraday 😆 but I am open to suggestions in how we can make Webmock a notch less... invasive. Maybe @bblimke has ideas?

[iMacTia]

Hi @julik, I did the same exact test on my Mac and I can see the body printed so I'd assume the issue lies on WebMock instead as you suggested. Sorry for not thinking about it straight away, I'll close this PR and open a new one against WebMock to get this fixed.

Unfortunately in my case I'm in the middle of rewriting Faraday tests using WebMock so it would be pointless to use WebMock against Faraday 😄, as I need to ensure that the Faraday API correctly maps calls to the underlying adapter.

As a quick-fix I'll just disable the tests assuming everything is actually working outside of WebMock, but hopefully we can get this fixed 👍

Previous tests were using a mock server like the one you set up but as you can imagine they were much more complex and, especially, slower than the equivalent using WebMock

[julik]

Glad you were able to reproduce. Sorry that I can't offer a better recipe for fixing this. If I could make a suggestion I would avoid using Webmock for Faraday itself because you are in a unique position of being an infrastructure library, so depending on something to make sure you work correctly might not be the best solution in this case. But that ofc rests on your decisions since you are the maintainer ;-) Let me know if we can be of any further assistance.

[iMacTia]

Thank you very much @julik, I appreciate your help and of course respect your opinion.

Main reason for me wanting to switch to WebMock is that tests were really messy and even the most basic scenarios were not tested against all adapters. The "Live Server" was also creating some issues on Travis and on local environments, plus making the tests quite slow.

At least using WebMock I can achieve a broader, faster test suite; even though I totally understand your concerns about dependencies. However I don't exclude the possibility of adding back a live server to the suite in the future or in case I hit any blocker using WebMock.