hayesdavis / grackle

Lightweight Ruby library for the Twitter API that goes with the flow.
228 stars 36 forks source link

extra /users gets added if call attempt is made, but fails because variable wasn't set prior #9

Open damon opened 14 years ago

damon commented 14 years ago

pretty odd.

u = client.users.show? :user_id => x NameError: undefined local variable or method `x' for #<Object:0x389a0 @controller=#> from (irb):8 x = 12 => 12

u = client.users.show? :user_id => x Grackle::TwitterError: get http://twitter.com/users/users/show.json?user_id=12 => 404: {"request":"\/users\/users\/show.json?user_id=12","error":"Not found"} from /Users/damon/rails/tf/vendor/gems/hayesdavis-grackle-0.1.4/lib/grackle/client.rb:240:in handle_error_response' from /Users/damon/rails/tf/vendor/gems/hayesdavis-grackle-0.1.4/lib/grackle/client.rb:218:inprocess_response' from /Users/damon/rails/tf/vendor/gems/hayesdavis-grackle-0.1.4/lib/grackle/client.rb:198:in call_with_format' from /Users/damon/rails/tf/vendor/gems/hayesdavis-grackle-0.1.4/lib/grackle/client.rb:142:inmethod_missing' from (irb):10 u = client.users.show? :user_id => x => #<Grackle::TwitterStruct....

It seems that because x wasn't set the first time through, the /users part of the path gets added twice.

hayesdavis commented 14 years ago

The reason this happens is that users gets called on client and added to the request path. The 2nd method, show?, is not actually called because the undefined variable error happens prior to sending show? to the Client. This leave the internal state of the request path as "/users".

You can fix this type of issue by calling client.clear which will clear the request path.

This is interesting though and makes me think that instead of using a single request path as an instance variable of the client and returning the client from the path component methods, I could create a proxy object that's returned by the path component methods that holds request state. That would prevent this confusing behavior from happening.

damon commented 14 years ago

sounds like a good change because /users by itself in this context is useless. they only make sense taken together, I think.

at least it's understood! :)