kirillplatonov / shopify_graphql

Less painful way to work with Shopify Graphql API in Ruby.
MIT License
59 stars 9 forks source link

Response Code 430 #38

Closed kgorshkov closed 4 months ago

kgorshkov commented 5 months ago

Hi,

re: commit 7c3ab7e83b78e15e6bf6e1d785942fe09c8722af 'Handle 430 error'

when 429, 430 raise TooManyRequests.new(parsed_body(response), code: response.code)

We have been receiving sporadic and brief bursts of 430 Responses for GraphQL Admin API calls in production in the past few months. They do not occur because there are too many calls within a given period of time in our case. I opened a ticket and here is a reply from Shopify Partner Support:

I understand the question and our API Response Codes dev doc does give more detail around the specific error. The 430 error is a Shopify Security Rejection which occurs when the request is deemed to be potentially malicious, and Shopify has responded by rejecting it to protect the app from any possible attacks.

and in a later reply:

So our team wasn't able to glean any real specifics regarding the cause of these 430 errors.... One thing they did mention is that the most common issue that comes up to cause this sort of behavior is if someone's app is deployed somewhere on a cloud platform that's using a public or shared IP address. Do you know if you would be using a Shared IP, or have you got a dedicated IP for this?

So the purpose of this Issue is twofold:

Thanks

kirillplatonov commented 5 months ago

Hi @kgorshkov,

I started receiving lots of 430 errors too lately with TOO_MANY_REQUESTS error code:

{"errors":[{"message":"You have sent too many requests. Try again soon.","extensions":{"code":"TOO_MANY_REQUESTS"}}],"error_reference":"If you report this error, please include this id: xxx"} (ShopifyAPI::Errors::HttpResponseError)

This seems to be triggered due to the shared IP on Heroku. Using static outbound IP would cost a fortune so I think retrying failed requests will be the easiest solution.

In my opinion Response 430 should remain under the ConnectionError umbrella rather than being thrown as TooManyRequests.

We could move it into separate kind of error (eg ShopifySecurityRejectionError). Treating it as ConnectionError wouldn't allow us to auto retry failed request.

It would be useful provide the headers of the response via the ConnectionError/ClientError/etc -- Shopify Partner Support needs the x-request-id from the response header to be able to investigate the exact cause of my 430 Responses via their logs. I imagine that access to the headers would be useful to others in the case of an error. (I know I can access the headers with a direct request using ShopifyAPI::Clients::Graphql::Admin instead of shopify_graphql).

This information is already available in error_reference when exception is raised (eg If you report this error, please include this id: xxx). You should see it in your error tracker.

kirillplatonov commented 4 months ago

Closing for now. Let me know if you'll have new input on this.