I just played around with commerce and I was evil :D
While playing around I deleted one of the referenced line items of an order. And as expected I got a "WSOD" because the entity metadata wrapper failed.
I know, under normal conditions this shouldn't happen, but for the sake of ultra defensive coding and usability (avoiding a WSOD is great usability ;) ) I thought I could add some "self healing".
I've added a check if a line item really exists in commerce_cart_order_refresh()
before cloning it. If this fails, the reference is removed (To properly work this needs following entity fix #1423174: Use array_key_exists() instead isset() in offsetExists()). That way the consistency of the references is restored.
Overhead of the whole thing should be insignificant.
Btw. the other two changes in the patch are only replaced tabs.
Comments
Comment #1
rszrama CreditAttribution: rszrama commentedSimple enough! Committed.
Comment #3
aidanlis CreditAttribution: aidanlis commentedThis doesn't just happen in the hypothetical - if you get to the "redirect to external payment" step, then use the browsers back button to click around, then add another item to your cart you'll WSOD. Thanks for the patch!
Comment #4
jsacksick CreditAttribution: jsacksick commentedI was having an issue with an order discount causing a fatal error and the similar fix applied to
commerce_order_calculate_total()
somehow did the trick.Related issues : #1905234: Commerce Discount cause an error when reusing shipping address for billing and #1408464: EntityMetadataWrapperException
Comment #5
jsacksick CreditAttribution: jsacksick commentedSame patch with the use of the
offsetUnset()
methodComment #6
jwjoshuawalker CreditAttribution: jwjoshuawalker commentedYes, yes, yes.
jsacksick++
I don't think this issue will get attention since the original was committed. I created a new post here and attached your patch, marking as RTBC:
#1993938: Add some self healing for order line item references (again)
Comment #7
rszrama CreditAttribution: rszrama commentedAny open issue will get addressed. No need to create a duplicate.
Comment #8
jwjoshuawalker CreditAttribution: jwjoshuawalker commented@rszrama
Sorry Ryan. Thanks for the attention on this one.
Comment #9
rockaholiciam CreditAttribution: rockaholiciam commentedencountered the problem where after updating cart the shipping checkbox would start throwing the commerce total exception and the patches provided fixed it. thanks.
Comment #10
joelpittet@jsacksick re #5 Why do you need to use offsetUnset() instead of unset()?
I may be dense, but isn't the purpose of that method to allow the magic __unset() to call it? Or is there some bug or performance gain from calling this directly?
Comment #11
mglamanPatch fixed my issue that was occurring when Commerce Discount discount active, explained in #1905234: Commerce Discount cause an error when reusing shipping address for billing
Comment #12
bojanz CreditAttribution: bojanz commentedThe wrapper currently doesn't implement the magic __unset method (even though it does implement __isset).
Comment #13
rszrama CreditAttribution: rszrama commentedCommitted, thanks guys.