Handle refresh_token, add GatewayTimeout error, store intuit_tid on errors, support custom middleware

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

Handle refresh_token, add GatewayTimeout error, store intuit_tid on errors, support custom middleware #88

Closed ntippie closed 4 years ago

ntippie commented 6 years ago

I have a number of changes from my fork that I wanted to get feedback on:

Support for OAuth2 refresh_token
- Initialize QboApi with refresh_token to have it manage access_token
- Set refresh_proc to process refresh_tokens returned when fetching an access_token
- Currently calls proc every time access token is fetched, even if refresh_token unchanged
- If access_token is expired, refreshes access_token and retries current request
Support for custom middleware
- Use qbo_api.middleware.prepend and qbo_api.middleware.append
Fix for constant defaults (#87)
Add GatewayTimeout error
Store intuit_tid in Error#fault

minimul commented 6 years ago

Thanks Nate. I'll be checking it out tomorrow.

minimul commented 6 years ago

With regards to the refresh token process can you post some code of it in action?

@bf4 You all are doing OAuth2 – what approach are you taking around renewing?

bf4 commented 6 years ago

@ntippie in general, awesome for the PR. Let's break it into smaller pieces so it's easier to review, and make sure we implement them in the example apps

I just pushed some unfinished work related to this pr and the example app for reference https://github.com/minimul/qbo_api/pull/89

bf4 commented 6 years ago

Let's break this into separate PRs

[ ] add hook to handle authorization error
- [ ] add oauth2 refresh middleware, include in example app
[ ] handle 504 (generally more customizable error handling)
[x] handle error body parse error
[ ] add more info to exception (intuit tid)
[ ] discuss changing include Connection to prepend

minimul commented 4 years ago

@ntippie I'd like to close this but to point others to your fork within the README. Is the fork ready to be pointed to?

ntippie commented 4 years ago

@minimul Yup, we are using it in production. Recommended to point to it as learning example since it may change unexpectedly.

minimul commented 4 years ago

Closing as I didn't go this route for OAuth2 renewal but if anyone is interested in this middleware renewal approach check out @ntippie latest commits above.