Problem using VCR and Instagram gem #159

Closed
jam1401 opened this Issue May 1, 2012 · 23 comments

Projects

None yet

4 participants

@jam1401
jam1401 commented May 1, 2012

I'm using the Instagram GEM in my application when I use VCR in my tests I run into problems and I have no idea how to solve the issue. I cant figure out if this is a VCR issue or not. Basically the initial request fills a cassette with the following data

http_interactions:
- request:
    method: get
    uri: https://api.instagram.com/v1/users/self/feed.json?access_token=xxxxxxxxx
    body:
      encoding: US-ASCII
      string: ''
    headers:
      Accept:
      - application/json; charset=utf-8
      User-Agent:
      - Instagram Ruby Gem 0.8.3
      Authorization:
      - Token token=xxxxxxx
  response:
    status:
      code: 200
      message: !!null 
    headers:
      content-language:
      - en
      content-type:
      - application/json; charset=utf-8
      date:
      - Tue, 01 May 2012 03:03:39 GMT
      server:
      - nginx/0.8.54
      vary:
      - Accept-Language, Cookie
      x-ratelimit-limit:
      - '5000'
      x-ratelimit-remaining:
      - '4995'
      content-length:
      - '2036'
      connection:
      - keep-alive
    body:
      encoding: US-ASCII
      string: ! '#<Hashie::Mash data=[#<Hashie::Mash attribution=nil caption=nil comments=#<Hashie::Mash
        count=0 data=[]> created_time="1335764579" filter="Normal" id="xxxxxxxxxxxxxx"
        images=#<Hashie::Mash low_resolution=#<Hashie::Mash height=306 url="http://distilleryimage4.instagram.com/5845149c928711e19e4a12313813ffc0_6.jpg"
        width=306> standard_resolution=#<Hashie::Mash height=612 url="http://distilleryimage4.instagram.com/5845149c928711e19e4a12313813ffc0_7.jpg"
        width=612> thumbnail=#<Hashie::Mash height=150 url="http://distilleryimage4.instagram.com/5845149c928711e19e4a12313813ffc0_5.jpg"
        width=150>> likes=#<Hashie::Mash count=1 data=[#<Hashie::Mash full_name="Rich
        .... >

And the tests pass without a problem. However subsequent requests that pull from the Cassette fail

NoMethodError: undefined method `each' for #String:0x007f9676681758

It seems that VCR is simply returning a string and the API just delivers it to the app and things fail

Any suggestions? been stuck on this for a while and cant seem to find a solution.

thanks

Jeff

Owner

The Hashie::Mash stuff in the response body looks suspicious/wrong. That's probably the source of what's going on. Hashie::Mash is a subclass of Hash and thus has an each method. VCR is intended to record the response as a raw string. Something weird is going on that's causing it to record the Hashie::Mash rather than the response body that the Hashie::Mash is parsed from.

Without example code that demonstrates the issue, it's hard to say for sure what the problem is...one thought, though: I wonder if the instagram gem is replacing the body attribute of the response object with a Hashie Mash, rather than wrapping the response object with an object that presents the hashie mash interface? Just one theory...

Can you come up with an example the demonstrates the issue? I should be able to get to the bottom of it with that, but I don't think I have the bandwidth to spend the time coming up with an example on my own (especially since I've never used the instagram gem).

Thanks!

jam1401 commented May 1, 2012

I tend to agree with you about the response being suspicious. I'll create an example a put up on github. Will let you know when it is available.

jam1401 commented May 2, 2012

Ok well I solved the problem, I was using hook_into: faraday in my configuration mainly because Instagram gem uses faraday. However this was the cause of the problem. Removing this configuration removed the problem I was having.

@jam1401 jam1401 closed this May 2, 2012
Owner

There's still a bug somewhere, though...hook_into :faraday is generally the right way to use VCR with a gem that is built on faraday. I'd still love to see some example code of the problem so I can look into fixing it!

jam1401 commented May 2, 2012

Yeah, I agree should look into this deeper. I still intend to provide a test case. My current problem is the no :faraday option fixes the instagram tests but when I change the configuration in spec_helper to reflect this some other vcr tests fail. I tried using tags and before/after record to remove faraday for just the instagram tests but that didn't work.

Let me work on the sample app and then you can probably figure it out better than I can

Owner

My current problem is the no :faraday option fixes the instagram tests but when I change the configuration in spec_helper to reflect this some other vcr tests fail. I tried using tags and before/after record to remove faraday for just the instagram tests but that didn't work.

yeah, the hook_into option is a one time thing; once you've added it, you can't remove it.

jam1401 commented May 3, 2012

Ok, try this repo

git@github.com:jam1401/instavcr.git

It will hopefully allow you to debug the issue. Let me know if you have questions or the repo doesn't work for you.

@myronmarston myronmarston reopened this May 4, 2012
Owner

Thanks for creating that! I'll try to take a look soon.

Owner

I don't have an instagram account...is there a way to repro the error w/o an instagram account?

jam1401 commented May 4, 2012

I'll create a test account

Thanks

Jeff

Sent from my iPad

On May 3, 2012, at 9:56 PM, Myron Marstonreply@reply.github.com wrote:

I don't have an instagram account...is there a way to repro the error w/o an instagram account?


Reply to this email directly or view it on GitHub:
https://github.com/myronmarston/vcr/issues/159#issuecomment-5504261

Owner

Actually, I think I just found the problem by looking at the source of the instagram gem...

jam1401 commented May 4, 2012

Uname vcrtest, same for pass

Email for account is jeff@themorganfamily.net

Cheers

Jeff

Sent from my iPad

On May 3, 2012, at 9:56 PM, Myron Marstonreply@reply.github.com wrote:

I don't have an instagram account...is there a way to repro the error w/o an instagram account?


Reply to this email directly or view it on GitHub:
https://github.com/myronmarston/vcr/issues/159#issuecomment-5504261

jam1401 commented May 4, 2012

Yeah, it looked to me like they were sending binary data but trying the same URL from chrome returned pure JSON of course documentation is sparse.

Unfortunately it's the only gem I could find

Sent from my iPad

On May 3, 2012, at 10:31 PM, Myron Marstonreply@reply.github.com wrote:

Actually, I think I just found the problem by looking at the source of the instagram gem...


Reply to this email directly or view it on GitHub:
https://github.com/myronmarston/vcr/issues/159#issuecomment-5504527

Owner

@jam1401 -- I'm still trying to figure out the best fix here, but I wanted to let you know what the underlying problem is. The instagram gem does something that's very non-standard: it puts the faraday HTTP adapter before a response-modifying middleware in the connection stack. As the Faraday README mentions, the adapter should be last unless you really have a good reason not to put it there.

VCR hooks into Faraday by inserting a middleware just above the adapter. It needs to be here so that it has "first-dibs" on the response (to record it exactly as-is) when the real request is made, and so it can intercept the request and return the previously recorded response on playback (before the request gets to the adapter). With the instagram faraday connection setup, it modifies the response before VCR has a chance to record it.

I need to run out the door now, but I'll try to follow up later with some ideas for potential solutions here.

Owner

So, I've got a few ideas of potential changes to VCR to address this:

  • Update VCR's Faraday middleware so that it ensures that the recording of the response happens before any other middleware has access to change it. I'm not yet sure if this is doable, and if it is, it will likely involve accessing faraday internals (and not simply public APIs).
  • Update VCR's faraday hook so that when it inserts the VCR middleware, it detects a case like the instagram gem (e.g. where there's middleware after the HTTP adapter), and moves the detected middleware so that it comes before the VCR middleware in the stack. I'd probably also have it issue a warning about the fact that VCR is having to mess with your middleware stack to insert itself properly.
  • Raise an error if VCR detects a middleware after the HTTP adapter as it can't guarantee it'll work properly. This won't make the instagram gem work with hook_into :faraday but it will at least make the underlying problem very clear (rather than getting an obscure error), and really, the instagram gem should probably change it's middleware stack to be more standard.

So...those are the options I see at the moment. I'll see what I can come up with. @mislav -- I'd be curious to hear your thoughts on this issue, since you've contributed to both Faraday and VCR and know both projects well. How do you think VCR should handle this?

jam1401 commented May 5, 2012

Well being selfish I would love to get a workaround, the middle option looks reasonable to me and might help to prevent other requests coming to you caused by this kind of configuration.

However it seems it would be prudent to also generate a request on the instagram repo to fix their code. I'll take a crack at that and generate a pull request on their repo

Owner

Yeah, I'm leaning towards the second option as well. One potential downside is that re-arranging the middleware may have unintended side effects. I think it'll be fine for middleware that just parse and replace the response body but it's hard to say for other kinds of middleware if it'll always work the same. That's why I was thinking it would print off a warning so users are aware of what's going on.

However it seems it would be prudent to also generate a request on the instagram repo to fix their code. I'll take a crack at that and generate a pull request on their repo

Yep, I agree.

Member
mislav commented May 6, 2012

My thoughts:

  1. Magic.
  2. ZOMG magic.
  3. Good idea.
  4. My idea: why doesn't VCR simply record the Mash into YAML without casting it to string first? Or, if VCR is strict on only storing strings, then raise an error when the response body to store isn't a string. However, this exception would still be more obscure than one proposed in 3).

The problem here is that Instagram uses Faraday wrong, sure. But it actually works for them. So lets see what's wrong on our end. The biggest problem I see here that VCR will happily cast a Mash into a string and then restore it back as a string, therefore actually changing the data it was supposed to cache.

@myronmarston myronmarston added a commit that referenced this issue May 7, 2012
@myronmarston myronmarston Print a warning when a non-standard Faraday stack is used.
When the Faraday stack includes a middleware after the HTTP adapter it gets "first dibs" on modifying the response and thus can prevent VCR from recording the response properly.  This is a problem with the instagram gem.

For #159.
e536a51
@myronmarston myronmarston added a commit that referenced this issue May 7, 2012
@myronmarston myronmarston Raise an error if a response is recorded with an invalid body.
This can occur when a non-standard Faraday stack is used, where a response-modifying middleware comes after the HTTP adapter.

For #159.
d924f66
Owner

Thanks for the feeback, @mislav. I decided to go with a two-part solution:

  • Print a warning if there any middleware come after the HTTP adapter in the Faraday stack. I was originally planning to raise an error but I realized that this may not always be a problem, so a warning seems better.
  • Raise an error if the object recorded as the response body is not a string.

BTW, you can still use VCR with a faraday-based gem that has this issue; just use a different hook_into option (depending on what Faraday HTTP adapter you're using). Faraday builds on top of those HTTP libraries so if you stub it directly at that level using WebMock or whatever it should still work fine.

@jam1401 -- Let me know if this works for you.

jam1401 commented May 7, 2012

Yes this will work for me. I plan to fork and update the Intagram gem to use faraday correctly, which should clear this up completely. Thanks for your help on this

@jam1401 jam1401 closed this May 7, 2012
inkstak commented Jul 10, 2013

Hi @myronmarston

Am trying to use VCR with a gem which is using a custom Faraday middleware: musicbrainz
Here is its middleware, and the client which is using it.

I don't understand how to

just use a different hook_into option (depending on what Faraday HTTP adapter you're using)

The middleware use the default Faraday adapter (seems to be Net::HTTP).
Webmock is already catching other Net::HTTP requests.
But when requests are emitted by the musicbrainz gem, I just got the warning and cannot succeed to stub them.

If the problem is not only on the gem side, how can I succeed to stub all outgoing requests ?

Owner

@blakink: to quote @mislav, one of the Faraday maintainers:

Adapters should come last and nothing should come after them. Lots of people new to Faraday trip on this (i.e. they put it in the middle or even the beginning).

(From lostisland/faraday#170 (comment))

musicbrainz is broken, because it is putting its middleware after the adapter. You should work on getting that fixed upstream, as there's not much I can do about users using Faraday wrongly :(.

inkstak commented Jul 11, 2013

Thanks for you lights, I'll try to fix it in my fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment