Faraday parsing errors even after #31 about desk

[alexanderdean]

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]

@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]

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]

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

[alexanderdean]

Amazing thanks!

[colinc]

colinc/desk@b39d9fa

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]

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]

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

[alexanderdean]

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

[alexanderdean]

@colinc any chance you can raise a PR?

[alexanderdean]

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]

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