Skip to content

Prevent fatal error when product template variable is a Product object#181

Open
boo-code wants to merge 1 commit into
PrestaShop:devfrom
boo-code:fix/footer-product-fatal-on-product-object
Open

Prevent fatal error when product template variable is a Product object#181
boo-code wants to merge 1 commit into
PrestaShop:devfrom
boo-code:fix/footer-product-fatal-on-product-object

Conversation

@boo-code

Copy link
Copy Markdown
Questions Answers
Description? HookDisplayFooterProduct::run() reads the product Smarty variable and passes it straight to ProductWrapper::prepareItemFromProduct(), which consumes it with array keys ($product['id_product'], $product['name'], …). That variable is normally a ProductLazyArray (array-accessible), but some themes/modules assign a raw Product object to it, producing a fatal error Cannot use object of type Product as array on the product page (reported on 5.0.3 / PS 8.2.4). This PR guards the hook: if the product is not array-accessible, it skips tracking instead of breaking the page. ProductLazyArray implements ArrayAccess, so the normal flow is unaffected.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #179.
How to test? On a product page where the product Smarty variable is a raw Product object, before this PR the page fatals with Cannot use object of type Product as array (ProductWrapper.php line ~136). Deterministic check: prepareItemFromProduct(new Product(1)) throws that error; with the guard, HookDisplayFooterProduct::run() returns without a fatal. With the standard ProductLazyArray (which implements ArrayAccess), tracking continues to work as before.
Sponsor company

HookDisplayFooterProduct::run() passes the 'product' Smarty variable straight
to ProductWrapper::prepareItemFromProduct(), which accesses it with array keys.
The variable is normally a ProductLazyArray (array-accessible), but some themes
or modules assign a raw Product object, causing a fatal error
"Cannot use object of type Product as array" on the product page.

Guard against a non-array-accessible product and skip tracking in that case
instead of breaking the whole product page.
@ps-jarvis

Copy link
Copy Markdown

Hello @boo-code!

This is your first pull request on ps_googleanalytics repository of the PrestaShop project.

Thank you, and welcome to this Open Source community!

@github-project-automation github-project-automation Bot moved this to Ready for review in PR Dashboard Jun 27, 2026

@Hlavtox Hlavtox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@boo-code I am don't have a great feeling from this change. I think we should not add checks for bad modules overwriting system variables..

Cc @kpodemski @Codencode

@kpodemski kpodemski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me the change is totally fine, and I'm always recommending to check whether the input data is what we expect.

@ps-jarvis ps-jarvis added the Waiting for QA Status: Action required, Waiting for test feedback label Jul 2, 2026
@ps-jarvis ps-jarvis moved this from Ready for review to To be tested in PR Dashboard Jul 2, 2026
@boo-code

boo-code commented Jul 3, 2026

Copy link
Copy Markdown
Author

Thanks @Hlavtox, that is a fair principle and I do not want this to become a pattern of guarding against every misbehaving module either. Let me explain why I think this specific case is worth it, and offer a middle ground.

The distinction I would draw: product here is not this module's own state, it is a shared Smarty template variable that the hook reads but does not set. It is populated by the theme, core, or another module, so from this hook's point of view its type is genuinely external input crossing a trust boundary, not a value we control. The guard validates input at that boundary rather than compensating for one known-buggy module.

The failure modes are also very asymmetric. Without the guard, a third party assigning a raw Product object produces an uncaught fatal that takes down the whole product page (issue #179 is exactly this, on a live shop), not just GA tracking. With the guard, the only consequence is that GA tracking is skipped for that one request. Degrading the analytics is proportionate; crashing the storefront is not.

It is also deliberately minimal and does not touch the normal path: ProductLazyArray implements ArrayAccess, so the standard flow is unaffected and only the anomalous type returns early.

On your underlying concern, that we should not silently mask a bad module: I agree, and I am happy to add a debug-level log when the guard trips, so the misconfiguration stays visible and diagnosable instead of being swallowed. If you would prefer that, I will push it; otherwise I am glad to leave the guard as it is. Either way, thanks for the careful read.

@Codencode

Copy link
Copy Markdown

Indeed, I also think it's better to avoid the page crashing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Waiting for QA Status: Action required, Waiting for test feedback

Projects

Status: To be tested

Development

Successfully merging this pull request may close these issues.

Fatal error

6 participants