Update BaseApiClient.php

[ZaneCEO]

Guzzle default behavior is to wait forever, without any timeout. From https://docs.guzzlephp.org/en/stable/request-options.html#timeout :

Use 0 to wait indefinitely (the default behavior).

Without a defined timeout, the application hangs if BigCommerce doesn't provide a response. I've seen an application remain open for days with an ESTABLISHED TCP connection to BigCommerce.

This edit fixes it, giving BigCommece 45 seconds to provide a response before a timeout is thrown.

[ZaneCEO]

hello @jswift ! Sorry to bother, but this issue is critical for our application. Can I expect this PR to be merged or should I fork the repo? Thanks.

[joelreeds]

The timeout should be configurable. One approach introduces a new function parameter to __construct() for array $options that can be used to pass options into the class. This would allow for future additions to the options set.

    public const DEFAULT_TIMEOUT = 'timeout';

    private array $options = [
        'guzzle' => [
            'timeout' => 45,
        ],
    ];

    public function __construct(
        string $storeHash,
        string $clientId,
        string $accessToken,
        ?\GuzzleHttp\Client $client = null,
        array $options = []
    ) {
        $this->storeHash    = $storeHash;
        $this->clientId     = $clientId;
        $this->accessToken  = $accessToken;
        $this->setBaseUri(sprintf($this->defaultBaseUrl(), $this->storeHash));

        $this->client = $client ?? $this->buildDefaultHttpClient();

        $this->options = array_replace($this->options, $options);
    }

    private function buildDefaultHttpClient(): \GuzzleHttp\Client
    {
        $history = Middleware::history($this->debugContainer);
        $stack   = HandlerStack::create();
        $stack->push($history);
        return new \GuzzleHttp\Client([
            self::DEFAULT_HANDLER  => $stack,
            self::DEFAULT_BASE_URI => $this->getBaseUri(),
            self::DEFAULT_TIMEOUT  => $this->options['guzzle']['timeout'],
            self::DEFAULT_HEADERS  => [
                self::HEADERS__AUTH_CLIENT  => $this->clientId,
                self::HEADERS__AUTH_TOKEN   => $this->accessToken,
                self::HEADERS__CONTENT_TYPE => self::APPLICATION_JSON,
                self::HEADERS__ACCEPT       => self::APPLICATION_JSON,
            ],
        ]);
    }

Now when instantiating the BC API client, we can pass in a timeout value override.

$options = [
    'guzzle' => [
        'timeout' => 15
    ]
];

$api = new BigCommerce\ApiV3\Client($_ENV['hash'], $_ENV['CLIENT_ID'], $_ENV['ACCESS_TOKEN'], null, $options);

[ZaneCEO]

@joelreeds Your solution is way better than mine (alas, a bit overkill). I hope one or the other will be merged, but my PR is 1 month old now and I got no reply from the dev team....

[joelreeds]

alas, a bit overkill

For this one change, absolutely! I'm thinking of ways to make adding these types of changes in the future easier and require little to no restructuring.

This is also something we require as well so fingers crossed that at least one of these approaches is implemented as a wide-open timeout is a bad practice.

[jswift]

My apologies, I've been caught up and didn't hand this project over to anyone else. I'm back onboard now and will be actively monitoring and implementing some of the new endpoints. I think @joelreeds 's idea of a default option set that can be overriden is the best option, I'll do that right now and add a default timeout as well.

[jswift]

I've made a pr that addresses this here: https://github.com/aligent/bigcommerce-v3-api-php-client/pull/173/