Shopify / shopify-api-php

MIT License
376 stars 172 forks source link

Library assumes all responses are JSON structured - they aren't #302

Closed mdisimino closed 4 months ago

mdisimino commented 11 months ago

Issue summary

Rest/Base.php's request() method assumes all responses to be JSONs whereas some of them are HTML.

        **if ($statusCode < 200 || $statusCode >= 300)**
        {
                $message = "REST request failed";

                **$responseBody = $response->getDecodedBody();**
                if (!empty($responseBody["errors"]))
                {
                        $bodyErrors = json_encode($responseBody["errors"]);
                        $message .= ": {$bodyErrors}";
                }

                throw new RestResourceRequestException($message, $statusCode);
        }

Eg. for error codes 502/503/504, and possibly others, response is HTML, eg. 502 Bad Gateway...

$response->getDecodedBody() will call json_decode() which will throw a JsonException which will prevent RestResourceRequestException from being thrown.

sebastianpisula commented 11 months ago
app.CRITICAL: JsonException: Syntax error in /vendor/shopify/shopify-api/src/Clients/HttpResponse.php:48 Stack trace: 
#0 /vendor/shopify/shopify-api/src/Clients/HttpResponse.php(48): json_decode('<!DOCTYPE html>...', true, 512, 4194304) 
#1 /vendor/shopify/shopify-api/src/Auth/OAuth.php(374): Shopify\Clients\HttpResponse->getDecodedBody() 
#2 /vendor/shopify/shopify-api/src/Auth/OAuth.php(136): Shopify\Auth\OAuth::fetchAccessToken(Array, Object(Shopify\Auth\Session)) 
#3 /vendor/company/shopify-bundle/src/Controller/Auth/AuthCallbackController.php(49): Shopify\Auth\OAuth::callback(Array, Array, Array) 
#4 /vendor/symfony/http-kernel/HttpKernel.php(163): Company\ShopifyBundle\Controller\Auth\AuthCallbackController->index(Object(Symfony\Component\HttpFoundation\Request), Object(Octolize\ShopifyBundle\Repository\ShopRepository), Object(Symfony\Bridge\Monolog\Logger), Object(Symfony\Component\EventDispatcher\EventDispatcher)) 
#5 /vendor/symfony/http-kernel/HttpKernel.php(74): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1) 
#6 /vendor/symfony/http-kernel/Kernel.php(184): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#7 /vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php(35): Symfony\Component\HttpKernel\Kernel->handle(Object(Symfony\Component\HttpFoundation\Request)) 
#8 /vendor/autoload_runtime.php(29): Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run() 
#9 /public/index.php(5): require_once('/...') 
#10 {main} {"shop":"....myshopify.com"}
github-actions[bot] commented 9 months ago

This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days.

sebastianpisula commented 8 months ago

I still have problems

mba242 commented 6 months ago

Same here.

github-actions[bot] commented 4 months ago

This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days.

github-actions[bot] commented 4 months ago

We are closing this issue because it has been inactive for a few months. This probably means that it is not reproducible or it has been fixed in a newer version. If it’s an enhancement and hasn’t been taken on since it was submitted, then it seems other issues have taken priority.

If you still encounter this issue with the latest stable version, please reopen using the issue template. You can also contribute directly by submitting a pull request– see the CONTRIBUTING.md file for guidelines

Thank you!

mderaspe commented 3 months ago

The problem persists. You might want to reopen.