Faraday parsing errors even after #31

alexanderdean commented 10 years ago

Even after the fixes set out in #31, I'm still getting Faraday::Error::ParsingError:

1.9.3-p545 :013 > results = Parallel.map((1000..100000), :in_threads=>30){|idx| Desk.case(idx) }
Faraday::Error::ParsingError: 757: unexpected token at 'Too Many Requests'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/json-1.8.1/lib/json/common.rb:155:in `parse'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/json-1.8.1/lib/json/common.rb:155:in `parse'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday_middleware-0.9.0/lib/faraday_middleware/response/parse_json.rb:11:in `block in <class:ParseJson>'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday_middleware-0.9.0/lib/faraday_middleware/response_middleware.rb:48:in `call'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday_middleware-0.9.0/lib/faraday_middleware/response_middleware.rb:48:in `parse'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday_middleware-0.9.0/lib/faraday_middleware/response_middleware.rb:39:in `process_response'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday_middleware-0.9.0/lib/faraday_middleware/response_middleware.rb:32:in `block in call'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday-0.8.9/lib/faraday/response.rb:63:in `on_complete'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday_middleware-0.9.0/lib/faraday_middleware/response_middleware.rb:30:in `call'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday-0.8.9/lib/faraday/response.rb:8:in `call'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday-0.8.9/lib/faraday/response.rb:8:in `call'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday-0.8.9/lib/faraday/request/url_encoded.rb:14:in `call'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday-0.8.9/lib/faraday/request/multipart.rb:13:in `call'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday_middleware-0.9.0/lib/faraday_middleware/request/oauth.rb:42:in `call'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/desk-1.0.0/lib/faraday/request/multipart_with_file.rb:16:in `call'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday-0.8.9/lib/faraday/connection.rb:253:in `run_request'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/faraday-0.8.9/lib/faraday/connection.rb:106:in `get'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/desk-1.0.0/lib/desk/request.rb:51:in `request'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/desk-1.0.0/lib/desk/request.rb:18:in `method_missing'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/desk-1.0.0/lib/desk/client/case.rb:18:in `show_case'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/desk-1.0.0/lib/desk.rb:25:in `method_missing'
    from (irb):13:in `block in irb_binding'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/parallel-1.0.0/lib/parallel.rb:389:in `call'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/parallel-1.0.0/lib/parallel.rb:389:in `call_with_index'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/parallel-1.0.0/lib/parallel.rb:229:in `block (3 levels) in work_in_threads'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/parallel-1.0.0/lib/parallel.rb:397:in `with_instrumentation'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/parallel-1.0.0/lib/parallel.rb:227:in `block (2 levels) in work_in_threads'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/parallel-1.0.0/lib/parallel.rb:221:in `loop'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/parallel-1.0.0/lib/parallel.rb:221:in `block in work_in_threads'
    from /home/vagrant/.rvm/gems/ruby-1.9.3-p545/gems/parallel-1.0.0/lib/parallel.rb:65:in `block (2 levels) in in_threads'1.9.3-p545 :014 > exit

alexanderdean commented 10 years ago

I've done some sleuthing and I think I know what is going on. I believe:

For rate limiting, Desk is returning a 429 with mime type JSON but a non-JSON body: "Too Many Requests"
Faraday is attempting to parse the non-JSON body as JSON and throwing an error
Faraday is not even reaching the desk gem's custom 4xx handler

colinc commented 10 years ago

@alexanderdean you are totally right and that was a known issue (that I totally forgot to go back and fix). That bug actually effects any time the Desk API has issues (so 500 errors, etc). If you're already working on fixing it then that's awesome, however, if you want some help, I have some time today that I can work on getting it fixed.

alexanderdean commented 10 years ago

Hi @colinc - a bit of help would be amazing - my Faraday-fu is not at all strong! Very happy to test anything you write...

colinc commented 10 years ago

@alexanderdean No problem. I've got a handful of commitments this morning and then I'll jump right on this.

alexanderdean commented 10 years ago

Amazing thanks!

colinc commented 10 years ago

colinc/desk@b39d9fa346356a405b3ad1ebe5371644870d86e9

This was basically a one line fix! I ripped out some of the old XML code since they're only using JSON responses now. I guess I need to go through and find any other unnecessary XML code and clean that up as well.

I'm glad I got into that code though because I think there's some improvements that can be made to optionally allow the Desk gem to handle the rate limiting itself.

In the meantime, @alexanderdean can you see if this all works for you?

Here's the test I was using to verify the changes @alexanderdean and I made.

(0..125).each do |i|
  begin
    Desk.cases
  rescue Desk::EnhanceYourCalm => e
    sleep(e.retry_after)
  end
end

alexanderdean commented 10 years ago

Confirmed - that's working great for me, huge thanks @colinc . Here is my test:

1.9.3-p484 :012 > require "parallel"
 => true
1.9.3-p484 :013 > results = Parallel.map((1000..100000), :in_threads=>30) do |idx|
1.9.3-p484 :014 >       begin
1.9.3-p484 :015 >           Desk.case(idx)
1.9.3-p484 :016?>       rescue Desk::EnhanceYourCalm => e
1.9.3-p484 :017?>         puts "Enhance your calm for #{e.retry_after}s"
1.9.3-p484 :018?>         sleep(e.retry_after)
1.9.3-p484 :019?>       end
1.9.3-p484 :020?>   end
Enhance your calm for 22s
Enhance your calm for 22s
Enhance your calm for 22s
Enhance your calm for 22s
Enhance your calm for 22s
Enhance your calm for 22sEnhance your calm for 22s

Enhance your calm for 22s
Enhance your calm for 22s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s
Enhance your calm for 21s

How do we get all these changes merged and a new release cut?

rguerrettaz commented 10 years ago

+1 for the new release. Running into same issue. Thanks for all the hard work guys!

alexanderdean commented 10 years ago

@colinc - you okay to raise a PR into this project with your fix?

alexanderdean commented 10 years ago

@colinc any chance you can raise a PR?

alexanderdean commented 10 years ago

Hey @chriswarren - there are some further changes in @colinc's branch - unfortunately I don't think he ever raised a PR but his fixes are needed too...

chriswarren commented 9 years ago

I think this is all merged in now, so I'm closing the ticket. If it's not let us know.

chriswarren / desk

Faraday parsing errors even after #31 #32