Bug: Fatal error in salesOrderBeforeSave event for deleted payment

[sreichel]

See; https://magento.stackexchange.com/questions/104468/getting-php-fatal-error-call-to-a-member-function-getmethodinstance-on-boolea

Possible fix: https://github.com/OpenMage/magento-lts/blob/1.9.3.x/app/code/core/Mage/Payment/Model/Observer.php#L46

Change:

if ($order->getPayment()->getMethodInstance()->getCode() != 'free') {

To:

if ($order->getPayment() && $order->getPayment()->getMethodInstance()->getCode() != 'free') {

[Flyingmana]

in which cases is a deleted payment to be expected?

[sreichel]

Not tested yet, but one (for me) reproducable case could be .... disable M2E Pro Extension and modify an order where payment method is m2e_payment. I'm pretty sure i've alread read about this case (realted to M2E).

Edit:

Just looking to the code, this is indeed a bug ...

[Flyingmana]

in this case you dont delete the payment methods with this code, you configure a pseudo method with the same type to avoid all the errors.(I know there are some errors in the admin area, if the payment method to the code is not there anymore)

I think it is a bad pattern to delete payments. Once there may be a lot of modules, which expect a payment to exist for an order. And you need to reserve it for quite some time, to being able to handle returns correctly.

So I continue asking, is there another case where a payment is expected to be deleted?

[colinmollenhour]

I think @sreichel's suggested case is valid since stores can be installed for many years and naturally things change.. New payment methods are adopted and old ones deprecated. Perhaps the original author quit supporting it or it was replaced with a different method due to PCI compliance changes, etc..

In my opinion if getPayment() method can return something other than a MSMO_Payment then every time it is used the return value should be checked before assuming that it is an object (unless for some reason that is guaranteed to be a safe assumption such as it was just set in the same request). This applies as a general rule for all methods that can return both objects and non-objects and in this case I think the suggested fix is appropriate.

[rafaelpatro]

Hi guys I was facing this issue frequently. I have applied a Try Catch and it works too. Thanks

[damien-biasotto]

Should it sill be open since PR #730 was merged?

[sreichel]

@damien-biasotto #730 is not related.

[addison74]

This report is almost 5 years old. If there is still a problem in OpenMage and having a solution a PR should be created.

[sreichel]

This report is almost 5 years old.

... and "we" complained about Magentos issue tracker ... :sunglasses: