Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

amazon.pay.renderButton: Cannot read properties of null (reading 'onClick') #1215

Open
SamJUK opened this issue Oct 2, 2023 · 7 comments
Open

Comments

@SamJUK
Copy link
Contributor

SamJUK commented Oct 2, 2023

What I expected

Frontend JS to run without error.

What happened instead

NewRelic reporting a high occurrence of JS errors:

  • Cannot read properties of null (reading 'onClick') within default/en_GB/Amazon_Pay/js/amazon-button.min.js

Looking at the JS file, we rely on the amazon.pay.renderButton method to always return a valid HTML element. Even if it throws a exception and fails to assign the property, the flow is never altered. Ideally we should abort the _draw method if we fail to render the button, or at least check to make sure we've initialised a HTMLElement successfully.

https://github.com/amzn/amazon-payments-magento-2-plugin/blob/spc/view/frontend/web/js/amazon-button.js#L176

Your setup

  • Magento version: 2.4.5-p3
  • Amazon Pay Extension Version: dev-spc
  • Magento Edition: Community
image
@SamJUK
Copy link
Contributor Author

SamJUK commented Oct 14, 2023

To add to this, we are noticing another Amazon Pay JS error within NewRelic.

https://github.com/amzn/amazon-payments-magento-2-plugin/blob/spc/view/frontend/web/js/amazon-button.js#L294
image

@sgabhart22
Copy link
Contributor

Hello @SamJUK ,

I was able to reproduce the first console error in Firefox only, and the second appears to be from a PDP page; the Amazon Pay button doesn't seem to currently be enabled on product pages on the live site, so I couldn't see that one. Nonetheless, it seemed like the button still rendered correctly and was functional as expected from the /checkout/cart page, even with the error.

The first error is a bit curious, since, if self.amazonPay button doesn't exist at that point, the Promise should have been rejected and that line shouldn't even execute (line 173 of the file you linked). For the second error, if New Relic is consistently reporting that buttonConfig evaluates to null at this point, we could potentially look into splitting this logic and using optional chaining. Something like

let payload = self?.buttonConfig?.createCheckoutSessionConfig?.payloadJson;
payload ??= '';
let cartIdNull = payload.includes('cartId":null');

The same allowance may need to be added on line 300 (self.buttonConfig.spc_enabled) if buttonConfig keeps turning up null here. If you have NR configured to a staging site as well, that would be an ideal place to test, but I'd be interested to hear any feedback regarding this -- or any another -- solution to reduce the noise in New Relic.

Thanks!
Spencer

@SamJUK
Copy link
Contributor Author

SamJUK commented Oct 17, 2023

Hi Spencer,

Regarding the first error, we trigger the rejection callback but never return / alter the execution flow. So it will just continue processing the rest of the code below. See the following snippet for an example.

(function() {
    let shouldReturn = false;
    const testPromise = (res, rej) => {   
        console.log('Test Promise Running');
        if (shouldReturn) {
            return rej('Rejected');
        } else {
            rej('Rejected');
        }
        
        console.log('Test Promise Ending');
        res('Resolved');
    }
    

    console.groupCollapsed('Testing No Return');
    testPromise(console.log, console.error);
    console.groupEnd('Testing No Return');

    
    shouldReturn = true;
    console.groupCollapsed('Testing With Return');
    testPromise(console.log, console.error);
    console.groupEnd('Testing With Return');
    
})()

As for the 2nd one, is this a potential race condition within the _create method? Should we be waiting for the _draw promise to resolve before trying to subscribe to the cart?

Thanks
Sam

@sgabhart22
Copy link
Contributor

sgabhart22 commented Oct 17, 2023

Hi again @SamJUK ,

Thanks for the feedback, I suppose I misspoke previously; we should not be executing line 173 in the first case, but clearly, we still are. For the second case, some type of race condition seems possible there.

On a related note, we are currently examining some options for restructuring some of the module's frontend code to make it more readable, maintainable, and performant. These issues you've raised are certainly cases to consider toward that end. I am curious, have you seen this affect user experience/functionality in any way? Or is this just creating the New Relic noise and possibly affecting apdex scores? If so, you may be able to temporarily ignore these specific errors in NR until proper fixes can be rolled out.

Thanks,
Spencer

@SamJUK
Copy link
Contributor Author

SamJUK commented Jan 3, 2024

Is there any movement on the restructuring?

Picking this back up after the holiday period, looking over Sentry & our session recordings. It does appear to be having an effect on conversions. In the few examples we've looked at, once customers encounter the error they get stuck in the checkout process unable to proceed before giving up.

image

@SamJUK
Copy link
Contributor Author

SamJUK commented Apr 26, 2024

Hi All,

Any updates on this? This is really starting to affect the feasibility of keep Amazon pay active on the production site.

Thanks
Sam

@sgabhart22
Copy link
Contributor

Apologies @SamJUK , it took unexpectedly longer to get the newest release live on the Marketplace. Version 5.17.0 is now available for installation, and includes the frontend refactoring which may positively affect the behavior you've been seeing. I've just opened the PR to merge these changes into the SPC branch, these should be available soon.

Thanks,
Spencer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants