minimul / qbo_api

Ruby JSON-only client for QuickBooks Online API v3. Built on top of the Faraday gem.
MIT License
85 stars 46 forks source link

Faraday::TimeoutError - Net::ReadTimeout and request_id #68

Closed JohnSmall closed 4 years ago

JohnSmall commented 6 years ago

I've been running along smoothly for a few weeks now, coding in earnest, and suddenly today I'm getting loads of timeout errors, which is a nuisance as I'm writing and deleting, so not getting a response is a big issue.

I see in the QBO documentation that if I add requestid parameter to the path then I can send the request again and it'll be idempotent. I also see in the QboAPI code that if I set QboApi.request_id = true then it'll add a request id to the path. But...

There's no way to resend the request without triggering a brand new call to finaliaize_path which will call uuid again and get a new uuid so the requestid parameter will be different each time. That means each POST will be treated as new, and I'll be creating say invoices with the same data over again. It's not idempotent.

without changing the code in the gem, how can I resend a request with the same request_id ?

JohnSmall commented 6 years ago

Ok, I can't find anything in the gem code to deal with timeout errors so I'll have to add it myself. But I'll need your advice on how to reorganise the code a bit

In this method, finalize_path is the bit that adds the request_id as a new uuid each time it's called.

`  def request(method, path:, entity: nil, payload: nil, params: nil)
        raw_response = connection.send(method) do |req|
          path = finalize_path(path, method: method, params: params)
           case method
              when :get, :delete
                   req.url path
             when :post, :put
                 req.url path
                  req.body = payload.to_json
              end
          end
       response(raw_response, entity: entity)
       end
`

Obviously if I get a Faraday::Timeout error and re-run the request it'll call finalize_path again, and get a new request_id.

The problem is that finalize_path is inside the block and to re-send the request with the same request_id it has to be outside the block. It would have to change to something like

`  def request(method, path:, entity: nil, payload: nil, params: nil)
         path = finalize_path(path, method: method, params: params)
        tries = @configured_max_tries || 0
        begin
            raw_response = connection.send(method) do |req|
                case method
                 when :get, :delete
                   req.url path
                 when :post, :put
                    req.url path
                    req.body = payload.to_json
                end
            end
           response(raw_response, entity: entity)
       rescue Faraday::TimeoutError 
            if tries > 0
                 tries -= 1
                retry
             else
                 raise
             end
        end
       end
    `

But how do I test it, and where would I put the specs in the suite?

minimul commented 6 years ago

I am going to be merging in @bf4 refactoring PRs first so I'll respond after.

bf4 commented 6 years ago

@JohnSmall Any thoughts with the newest version?

fyi, I want to make the error handling more configuralbe as well

minimul commented 4 years ago

This gem will not be handling this error.

It's my recommendation that each integration project have a request_handler like approach so Faraday::Timeout can be handling there.