search

netresearch / module-shipping-core

The Netresearch shipping core package is the central component to offer generic functionality as required by all carriers.
Open Software License 3.0
0 stars 1 forks source link

Adding error handling #2

Closed DavidLambauer closed 3 years ago

DavidLambauer commented 3 years ago

We track more and more errors in sentry where $childItem is false and therefore the following code in readAttribute() is going to fail with Call to a member function getProduct() on bool. I didn't debug the whole stack trace yet. Nevertheless, the current function can return a bool and therefore the code afterwards won't succeed.

Would appreciate a quick merge.

mam08ixo commented 3 years ago

May I ask in which context you use this package? Chances are that a PR to this package / this package version will not fix it for you (see also dependency graph).

DavidLambauer commented 3 years ago

We're using this package in our Magento Shop https://github.com/netresearch/dhl-shipping-m2 which refers to this repo.

mam08ixo commented 3 years ago

Not quite. It's this one: https://github.com/netresearch/dhl-module-shipping-core

Sorry for the confusion. However, I understand the issue and we will take care of it.

DavidLambauer commented 3 years ago

Well, stupid sentence. We include netresearch/dhl-shipping-m2 which then refers to netresearch/module-shipping-core 😄

DavidLambauer commented 3 years ago

@mam08ixo here is the sentry reference in case you want to dig deeper: https://sentry.io/share/issue/de15009ffb49468fb797de8efaf6389e/

mam08ixo commented 3 years ago

Before we patch it around: Any idea how $children of a configurable item can be empty, leading to the error? It should contain the selected simple item.

DavidLambauer commented 3 years ago

As I said, we're having this issue for a couple of days. As far as we can see, this only happens to products that are out of stock. This issue only occurs by creating orders via admin panel.

mam08ixo commented 3 years ago

Still no luck replicating it. We will provide a workaround but you may experience other problems if the error turns out to be a symptom of some other issue.

mam08ixo commented 3 years ago

Executing composer update --with-all-dependencies dhl/shipping-m2 should pull in the shipping core 1.1.6 dependency. Give it a try.

mam08ixo commented 3 years ago

Since you were asking for a quick solution, any feedback on the fix?

DavidLambauer commented 3 years ago

Hi there,

we haven't seen any issues since this fix. Seems to be good. Thanks!