search

ant-hq / magento-module

MIT License
0 stars 0 forks source link

Gracefully handle corrupt magento products so GET products doesnt fail when there is one bad product #5

Open achadee opened 6 years ago

achadee commented 6 years ago

instead of returning 

<b>Fatal error</b>: Call to a member function getId() on a non-object in <b>/domains/achillesheel.co.uk/___cap/releases/20180608000213/magento/app/code/core/Mage/Catalog/Model/Product/Type/Configurable/Price.php</b> on line <b>85</b><br/>

return

products: [
{
   id: null
   error: "Call to a member function getId() on a non-object"
},
{
  id: 3,
  name: "test product"
  ....
}
PivitParkour94 commented 5 years ago

Untested code, this should be tested thoroughly with corrupt product data and safe product data to ensure it still works, and has solved the issue.

Do not merge this into Master until it has been tested.

Branch created https://github.com/ant-hq/magento-module/tree/issue_5-gracefully-handle-corrupt-magento-products

PivitParkour94 commented 5 years ago

May need to use try catches in all other places the setTheHashProductSimple and setTheHashConfigruableProduct functions are called.

For a start. That's only for the product data, if there are corrupt customer and order data, those won't be caught. This is definitely lending itself to having an Ant Model, that can return the failed object if any data is corrupt, however that might be too large a refactor for Magento 1, we should definitely do it with the Magento 2 module though.

achadee commented 5 years ago

So ideally for webhooks I would love to have a table thats setup that stores the last 1000 webhook events and stores the stack trace as a record. So if it succeeds GREAT! but if it fails we can tell our users to look somewhere (maybe even a UI to go with it!).

With GET /products we return an array of errors as part of the response from an import in Ant. So I can parse the stacktrace and present it in our UI.

PivitParkour94 commented 5 years ago

So are you saying there is no issue here? You are already getting an array of errors as part of the response. As far as I understood it you wanted the resulting product array to show either

  1. product data
  2. the exception message and have that all wrapped up in the product data from the API request.

If that's the case then having an array of errors doesn't really make sense. Please clarify what it is you exactly need.

The webhooks refactor could definitely be changed, however it would make sense to have that comment in issue #4

achadee commented 5 years ago

No there is still an issue, I just described what ideally I would like to happen.

So what currently happens on an import, if we query 50 products from page 2 for example

/products/?page=2&limit=50

if one of those products is corrupt for whatever reason we get an exception like this for example:

<b>Fatal error</b>: Call to a member function getId() on a non-object in <b>/domains/achillesheel.co.uk/___cap/releases/20180608000213/magento/app/code/core/Mage/Catalog/Model/Product/Type/Configurable/Price.php</b> on line <b>85</b><br/>

which means the other 49 products that arn't corrupt on the same query don't come into ANT.