Don't re-use empty body on retry #255
Conversation
There was a problem hiding this comment.
Thanks for the PR!
I left a couple of questions.
One request: can you add a request that triggers this behavior and confirms that the body is posted correctly on subsequent retries?
I think this could be done via httptest.NewServer() and capturing the request data.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #255 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 51 51
Lines 2191 2203 +12
=========================================
+ Hits 2191 2203 +12 ☔ View full report in Codecov by Sentry. |
|
Thanks for updating that! CodeCov indicates that not all new changes are covered, can you have a look plz? It might be the err != nil check for ReadAll(). Not sure how to test that but I don't want move away from the 100% test coverage so quickly. |
|
Yep, it was never hitting the error path for |
|
The tests still fail with an error, not entirely sure what the issue is. |
Retries on 429 (rate limit) were not working for me (except for GET requests). I would get these errors (retries result in a 400 Bad Request every time): [DEBUG] PUT: https://mydomain.myshopify.com/admin/api/2023-07/metafields/30029242335523.json [DEBUG] SENT: {"metafield":{"created_at":"2023-08-30T19:55:54-06:00","description":"The SKU of the product.","id":30029242335523,"key":"sku","namespace":"ns","owner_id":8557574947107,"own er_resource":"product","updated_at":"2023-08-30T19:55:54-06:00","value":"8bp-101009","type":"single_line_text_field","admin_graphql_api_id":"gid://shopify/Metafield/30029242335523"}} [DEBUG] Shopify X-Request-Id: 6676e531-ee1e-4efa-96b2-88682553f930 [DEBUG] RECV 429: 429 Too Many Requests [DEBUG] RESP: {"errors":"Exceeded 2 calls per second for api client. Reduce request rates to resume uninterrupted service."} [DEBUG] rate limited waiting 2s [DEBUG] Shopify X-Request-Id: [DEBUG] RECV 400: 400 Bad Request [DEBUG] RESP: <html> <head><title>400 Bad Request</title></head> <body> <center><h1>400 Bad Request</h1></center> <hr><center>cloudflare</center> </body> </html> The problem is that on retry, the same request with an already-read Body was being sent. The solution that is working for me is to copy the Body into a []byte buffer before the first request, and then set the req.Body to that buffer before every request.
|
Oops, that's |
Retries on 429 (rate limit) were not working for me (except for GET requests). I would get these errors (retries result in a 400 Bad Request every time):
The problem is that on retry, the same request with an already-read Body was being sent. The solution that is working for me is to copy the Body into a []byte buffer before the first request, and then set the req.Body to that buffer before every request.