-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Installing Twitter Gem breaks Savon due to HTTPI LOAD_ORDER preferring http over net_http #156
Comments
Also most probably relates to #16 which would have been a time saver if the right error message would have been thrown! |
relates more to #148. You can change the adapter that Savon uses though, as explained in comments on other issue: client = Savon.client(
...
:adapter => :net_http
) Seems as though this headers issue is an issue with the 'http' adapter; not sure what version of the 'http' gem @Connorhd developed the adapter against? |
@mikecmpbll what is the recommended adapter? Couldn't the team of savon just select one as dependency and be done with it? It certainly is the abstraction that caused the error but I am sure there is a reason for that. The time consuming part was making sense of the error message and the close to "non-existing stacktrace" about what caused the issue and from what gem it stemmed from. and yeah #148 seems to be the issue, but not under the update condition. However savonrb/savon#488 showed me the way on how to resolve it and how to make sense of it all. |
@lustremedia the LOAD_ORDER that you mentioned specifies HTTPI's adapter preference order, however you can use whichever adapter works best for you. HTTPI is a single interface to the myriad of http adapters available. The reason for your error appears to be an issue with the adapter for the http.rb gem, but I haven't had time yet to look in to it. The error almost certainly stems from this line of the adapter, though: https://github.com/savonrb/httpi/blob/master/lib/httpi/adapter/http.rb#L72 so it should be fairly easy for someone to work out what's up. The update itself isn't a condition in #148–the conditions are version 2.4.1 of httpi which has the http.rb support, and having the http gem installed. FWIW, I understand that Savon 3 will be dropping the HTTPI dependency and leaving it up to Savon end-users to extend their favourite HTTP library. |
@mikecmpbll As you could see from my incident description, I resolved the issue but wanted to let you guys know about. I searched the web and even this repo but #148 did not come up for me. Documentation about :adapter switch in savon is also nowhere to be found. Thanks for taking your time however to point out more about the issue and dropping the HTTPI dependency would be less headaches in the future I think. Cheers |
Hi @lustremedia. Sorry for the trouble. I don't know what httpi can do to help on these cases. Maybe savon could log what adapter is using or something like this. @mikeantonelli thanks for helping! 👍 |
@lustremedia thanks for reporting it, definitely something here that needs to be fixed so I'll see if I can check it out this evening. and yep, for some reason the adapter option is not documented for Savon.. |
Hi @rogerleite As I suggested: pick a single production ready adapter for savon and ship it with savon as dependency, but keep the option to change the adapter in the client if the developer wants something different. In that case, a well tested adapter is always present as default. If the developer chooses to use a different adapter but the adapter is broken he/she can report a bug with the adapter and savon would not be affected, but could point to the default adapter. @mikecmpbll 👍 Thanks for the help! And thank you both for the great and fast responses! 👍 |
@rogerleite also you could have the option to reject support for less active adapters and have less troubles. |
@lustremedia i agree with you. Seeing savonrb/savon#488, this suggestion can avoid a lot of problem, you can open an issue on savon. Thanks for reporting! I'm copying @tjarratt to be aware of this. |
@rogerleite see savonrb/savon#710 cheers |
Sorry for causing pain here since its my adapter thats not working, the problem is you have an older version of the http gem. If you update to a more recent version it should work. |
@Connorhd np! See, if I get the newest http gem it might break the twitter gem since the twitter gem has the http gem as dependency locked at a certain version which seems to work for them. Maybe they have already updated, but this triggered the savon LOAD_ORDER and broke Savon ... |
It looks like the next version of the twitter gem uses newer http, obviously that doesn't help you now :( I wonder if httpi should allow adapters to specify extra requirements to be enabled, so a minimum version could be given. |
@Connorhd I think if part of your code plays such an important role as to be a showstopper, maybe it is time for savon to decide on one http client and be done with it, rather than supporting all http clients and trying to figure out a LOAD_ORDER of the least client that is going to break. |
That's a discussion for over at savon. As far as HTTPI is concerned, I agree that adapters should be able to specify version dependency for their respective libraries. |
Using savon (2.11.1)
Using httpi (2.4.1)
Using twitter (5.14.0)
Using http (0.6.4)
I just installed the
twitter
Gem (https://github.com/sferik/twitter) which hashttp
Gem as dependency while using thesavon
Gem .HTTPI then preferred
http
over the workingnet_http
(this seems to be the default in Rails) in the LOAD_ORDER which then breaks Savon's requests with following error:NoMethodError (undefined method
headers' for #HTTP::Client:0xbd8ab91c):`After installing
httpclient
everything worked fine again due to the LOAD_ORDERhttps://github.com/savonrb/httpi/blob/master/lib/httpi/adapter.rb#L16
This seems to be related to savonrb/savon#488
The error messages are not very clear and the stacktrace for debugging was really non-existing but pointing to the Savon API calls. I think this ought to be remedied in some way.
The text was updated successfully, but these errors were encountered: