amzn / amazon-payments-magento-2-plugin

Extension to enable Amazon Pay on Magento 2
https://amzn.github.io/amazon-payments-magento-2-plugin/
Apache License 2.0
109 stars 77 forks source link

`Class i does not exist` on complete endpoint #1118

Closed haelbichalex closed 2 years ago

haelbichalex commented 2 years ago

What I expected

I expect to get a real error message when using a declined credit card or similar.

What happened instead

When using the /V1/amazon-checkout-session/:amazonSessionId/complete endpoint, I get an error from Magento when I should get an error from the module instead.

{
  "message": "Class i does not exist",
  "code": -1,
  "trace": "#0 /var/www/html/vendor/magento/framework/Reflection/MethodsMap.php(166): ReflectionClass->__construct('i')\n#1 /var/www/html/vendor/magento/framework/Reflection/MethodsMap.php(115): Magento\\Framework\\Reflection\\MethodsMap->getMethodMapViaReflection('i')\n#2 /var/www/html/vendor/magento/framework/Reflection/DataObjectProcessor.php(83): Magento\\Framework\\Reflection\\MethodsMap->getMethodsMap('i')\n#3 /var/www/html/vendor/magento/framework/Webapi/ServiceOutputProcessor.php(118): Magento\\Framework\\Reflection\\DataObjectProcessor->buildOutputDataArray(Object(Magento\\Framework\\Phrase), 'i')\n#4 /var/www/html/vendor/magento/framework/Webapi/ServiceOutputProcessor.php(78): Magento\\Framework\\Webapi\\ServiceOutputProcessor->convertValue(Array, 'int')\n#5 /var/www/html/vendor/magento/module-webapi/Controller/Rest/SynchronousRequestProcessor.php(97): Magento\\Framework\\Webapi\\ServiceOutputProcessor->process(Array, 'Amazon\\\\Pay\\\\Api\\\\...', 'completeCheckou...')\n#6 /var/www/html/vendor/magento/module-webapi/Controller/Rest.php(188): Magento\\Webapi\\Controller\\Rest\\SynchronousRequestProcessor->process(Object(Magento\\Framework\\Webapi\\Rest\\Request\\Proxy))\n#7 /var/www/html/vendor/magento/framework/Interception/Interceptor.php(58): Magento\\Webapi\\Controller\\Rest->dispatch(Object(Magento\\Framework\\App\\Request\\Http))\n#8 /var/www/html/vendor/magento/framework/Interception/Interceptor.php(138): Magento\\Webapi\\Controller\\Rest\\Interceptor->___callParent('dispatch', Array)\n#9 /var/www/html/vendor/magento/framework/Interception/Interceptor.php(153): Magento\\Webapi\\Controller\\Rest\\Interceptor->Magento\\Framework\\Interception\\{closure}(Object(Magento\\Framework\\App\\Request\\Http))\n#10 /var/www/html/generated/code/Magento/Webapi/Controller/Rest/Interceptor.php(23): Magento\\Webapi\\Controller\\Rest\\Interceptor->___callPlugins('dispatch', Array, Array)\n#11 /var/www/html/vendor/magento/framework/App/Http.php(116): Magento\\Webapi\\Controller\\Rest\\Interceptor->dispatch(Object(Magento\\Framework\\App\\Request\\Http))\n#12 /var/www/html/generated/code/Magento/Framework/App/Http/Interceptor.php(23): Magento\\Framework\\App\\Http->launch()\n#13 /var/www/html/vendor/magento/framework/App/Bootstrap.php(264): Magento\\Framework\\App\\Http\\Interceptor->launch()\n#14 /var/www/html/pub/index.php(29): Magento\\Framework\\App\\Bootstrap->run(Object(Magento\\Framework\\App\\Http\\Interceptor))\n#15 {main}"
}

The completeCheckoutSession method returns the result array directly, instead of wrapping it in another array, compared to most other methods in the CheckoutSessionManagement class. Also the method has a return type of int, which is not correct as far as I understand.

Steps to reproduce the issue

Your setup

sgabhart22 commented 2 years ago

Hello @haelbichalex ,

Thank you for raising the issue, you've brought up a couple of interesting points here. First, I think it would make sense to wrap the response(s) from completeCheckoutSession as you said, both for consistency and the convenience of accessing returned values by key instead of position when using the API. Second, the return type of int in the docblock causes a curious behavior when returning an array that contains an object (a \Magento\Framework\Phrase as error message here) via Magento's web API.

The remediation wouldn't be overly invasive, but it might affect some merchants currently using the endpoints in PWA setups, causing them to handle new response formats. We'll discuss some options and let you know what we come up with. Thanks again!

Spencer

sgabhart22 commented 2 years ago

Hello again @haelbichalex ,

We're currently reviewing a solution that will return the failure messages as pre-translated strings instead of Magento Phrase objects. This will keep the current implementation backwards compatible for PWA merchants who are already using the API, improve error messages for new and existing PWA merchants, and also not affect the standard frontend behavior for payment error messages.

You can find the PR here, please feel free to test from this branch (or with the attached patch file) and let us know if it's helpful!

gh-1118-patch.txt

Thanks, Spencer

sgabhart22 commented 2 years ago

Hi @haelbichalex ,

Curious if you've had the chance to test the changes from the previously provided PR/patch? We're planning on including it in an upcoming release, and it would be great to know if the changes worked OK for you!

Thanks, Spencer

haelbichalex commented 2 years ago

Hi @sgabhart22 ,

sorry for the late answer, I was busy fixing some other things with our integration first.

I installed the patch and everything seems to work just fine, the API returns the desired error messages 👍 Waiting for a new release now 🙂

Regards, Alex

sgabhart22 commented 2 years ago

@haelbichalex Glad to hear it! There should be a new one coming sooner rather than later, but we'll let you know again once it's live.

zichicc commented 2 years ago

Hi @haelbichalex , the new release (5.13.0) is live : https://github.com/amzn/amazon-payments-magento-2-plugin/releases/tag/5.13.0 please reopen this issue in case you have any further problems.

Thanks a lot Best Christian