Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resend requests that returns http error #240

Merged
merged 2 commits into from
Feb 23, 2022

Conversation

fabio-sassi-spotview
Copy link
Contributor

Sometime for temporary issue, shopify http request fail, for example I found in my test logs:

Request failed with HTTP Code 520
Request failed with HTTP Code 503
OpenSSL SSL_read: SSL_ERROR_SYSCALL, errno 104

If you try again a moment later, same requests are ok.

This implementation try to mitigate this problem.

I add a configuration parameters RequestRetryCallback that allow to define a callback to manage or log the retry.
The callback take 3 input parameter: raw curl response, exception object and retry number.
If this callback return true (or non null value) another attempt will be performed.

By default this callback is not defined and this does not change the original behavior of the library.

For example

PHPShopify\ShopifySDK::config([
    'ShopUrl' => SHOPIFY_URL,
    'AccessToken' => SHOPIFY_API_PASSWORD,
    'RequestRetryCallback' => function($response, $error, $retry) {
        echo "retry n=$retry: " . $error->getMessage() . "\n";
        sleep(1); 
        return $retry <= 5;
    }
]);

This is a modified version of #234 after @tareqtms advice.

@fabio-sassi-spotview
Copy link
Contributor Author

Hi @tareqtms

Any news about this pull request? Do you think it would be ok or it need of some refactory?

@tareqtms
Copy link
Contributor

@fabio-sassi-spotview
Sorry, I need to test GraphQL manually as there is no test case made for that yet. Just to make sure it didn't break. I will do it by tomorrow (hopefully).

@fabio-sassi-spotview
Copy link
Contributor Author

Hi @tareqtms

Have you any news?

@tareqtms tareqtms merged commit 3609f49 into phpclassic:master Feb 23, 2022
@tareqtms
Copy link
Contributor

@fabio-sassi-spotview
Merged and released. Sorry for the delay and thanks for your patience.

@fabio-sassi-spotview
Copy link
Contributor Author

thank you very much @tareqtms

@fabio-sassi-spotview fabio-sassi-spotview deleted the fault_tollerant_req branch February 24, 2022 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants