dotmailer / dotmailer-magento2-extension

The official Dotdigital for Magento2 extension
https://dotdigital.com/integrations/magento
MIT License
48 stars 63 forks source link

Invalid Return Type Breaks Swagger Interactive API Documentation Page #557

Closed floorz closed 4 years ago

floorz commented 4 years ago

Issue Magento Webapi processor validates return types on interfaces, and I don't think \DateTime is a valid return type. I didn't dig too deep, but it doesn't look like this CouponAttributeInterface is actually exposed over the webapi so I'm not sure why it's being validated by the api processor, but it is nonetheless.

Error Causing Line https://github.com/dotmailer/dotmailer-magento2-extension/blob/baa953568066fb1bd396cc4a9ba4e3ed673815b2/Api/Data/CouponAttributeInterface.php#L28

Resolution I know the code might actually return a \DateTime, but changing the return type to string|null fixes the error in Swagger. I'll leave it up to you how you want to fix, but hopefully this helps.

diff --git a/vendor/dotmailer/dotmailer-magento2-extension/Api/Data/CouponAttributeInterface.php b/vendor/dotmailer/dotmailer-magento2-extension/Api/Data/CouponAttributeInterface.php
index 1d64a4f..14efa0b 100644
--- a/vendor/dotmailer/dotmailer-magento2-extension/Api/Data/CouponAttributeInterface.php
+++ b/vendor/dotmailer/dotmailer-magento2-extension/Api/Data/CouponAttributeInterface.php
@@ -25,7 +25,7 @@ interface CouponAttributeInterface extends \Magento\Framework\Api\ExtensionAttri
     public function setEmail($email);

     /**
-     * @return \DateTime|null
+     * @return string|null
      */
     public function getExpiresAt();
sta1r commented 4 years ago

@floorz thanks for raising. Please can you explain how to replicate the original error? I've been to <magento-host>/swagger and get a different error.

floorz commented 4 years ago

@sta1r Here is what I have. Let me know if you need any other details

Configuration:

Deploy Mode: Production

In our composer.json it is aliased as version 4.1.0:

...
"require": {
...
        "dotmailer/dotmailer-magento2-extension": "4.3.6 as 4.1.0",
        "magento/product-enterprise-edition": "2.3.4"
    },
...

Other than that, I'm not doing anything special. A task I was working on required me to use Swagger to document some api endpoints and it didn't work due to the error. I checked the logs in var/log/exception.log

Error:

[user@host current]$ cat var/log/exception.log | grep 'webapi-5e7a65f073eb6'
[2020-03-24 19:56:32] report.CRITICAL: Report ID: webapi-5e7a65f073eb6; Message: The "\DateTime" parameter type is invalid. Verify the parameter and try again. {"exception":"[object] (Exception(code: 0): Report ID: webapi-5e7a65f073eb6; Message: The \"\\DateTime\" parameter type is invalid. Verify the parameter and try again. at /chroot/home/user/deploy/releases/7/vendor/magento/framework/Webapi/ErrorProcessor.php:208, InvalidArgumentException(code: 0): The \"\\DateTime\" parameter type is invalid. Verify the parameter and try again. at /chroot/home/user/deploy/releases/7/vendor/magento/framework/Reflection/TypeProcessor.php:435)"} []
[2020-03-24 19:56:32] report.CRITICAL: Report ID: webapi-5e7a65f073eb6; Message: The "\DateTime" parameter type is invalid. Verify the parameter and try again. {"exception":"[object] (Exception(code: 0): Report ID: webapi-5e7a65f073eb6; Message: The \"\\DateTime\" parameter type is invalid. Verify the parameter and try again. at /chroot/home/user/deploy/releases/7/vendor/magento/framework/Webapi/ErrorProcessor.php:208, InvalidArgumentException(code: 0): The \"\\DateTime\" parameter type is invalid. Verify the parameter and try again. at /chroot/home/user/deploy/releases/7/vendor/magento/framework/Reflection/TypeProcessor.php:435)"} []
sta1r commented 4 years ago

@floorz I found the error. Hitting /swagger indicates the error when loading (or processing) /rest/all/schema?services=all. I guess our interface gets processed there because it extends \Magento\Framework\Api\ExtensionAttributesInterface, and therefore must adhere to the same rules.

According to these service interface requirements, \DateTime is not a valid type, as you suggest.

I'll go ahead and merge your PR now - thanks for the contribution!