Skip to content

[4.x] Auto retry requests where the cart has expired#1236

Open
Jade-GG wants to merge 4 commits into4.xfrom
feature/auto-retry
Open

[4.x] Auto retry requests where the cart has expired#1236
Jade-GG wants to merge 4 commits into4.xfrom
feature/auto-retry

Conversation

@Jade-GG
Copy link
Copy Markdown
Collaborator

@Jade-GG Jade-GG commented Mar 18, 2026

ref: RAP-1854, BORDEX-1358

Currently, when your cart expires you will sometimes get an ugly and scary error telling you your cart ID is not valid.

With this PR, I check for this error and underwater we retry the request once, adding a more friendly notification telling you that your cart has expired in the meantime. This means that, for example, if you add a product to your expired cart, it will make a new cart and then re-add the product to this new cart.

This makes the user experience for expired carts a bit nicer.

Comment thread resources/js/fetch.js

return await window.magentoGraphQL(
query,
{ ...variables, cartId: mask.value, cart_id: mask.value },
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately some requests have cartId and others have cart_id. This was the simplest and most readable way to catch both.

indykoning
indykoning previously approved these changes Mar 23, 2026
Copy link
Copy Markdown
Member

@indykoning indykoning left a comment

Choose a reason for hiding this comment

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

Code wise it's looking good.
But perhaps we can simplify it a lot by doing what we are doing in the useUser store:

window.setTimeout(() => refresh(), 200)

I reckon refreshing the cart after loading the page will tackle most of the problems users experience.

@Jade-GG
Copy link
Copy Markdown
Collaborator Author

Jade-GG commented Mar 31, 2026

Code wise it's looking good. But perhaps we can simplify it a lot by doing what we are doing in the useUser store:

window.setTimeout(() => refresh(), 200)

I reckon refreshing the cart after loading the page will tackle most of the problems users experience.

This would move the error to the page load instead of the add to cart. It'd probably be cleaner, but it would not solve the issue of the users getting put off by a weird error.

We could perhaps combine the two to make the error silent on page load and immediately create a new cart when necessary?

@indykoning
Copy link
Copy Markdown
Member

I agree, we probably should silence the error if it happens during the "page load request"
And if it is done on page load we don't necessarily need to immediately create a new cart, as the next interaction with the cart (Add to cart) would create the new cart. I reckon there also wouldn't be any harm if a new cart would get created right away

indykoning
indykoning previously approved these changes Apr 1, 2026
Copy link
Copy Markdown
Member

@indykoning indykoning left a comment

Choose a reason for hiding this comment

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

Your tests are failing, other than that LGTM

@Jade-GG
Copy link
Copy Markdown
Collaborator Author

Jade-GG commented Apr 3, 2026

Your tests are failing, other than that LGTM

Yeah something with text rendering going weirdly, I'm not sure why. The part where I changed the playwright tests didn't do anything to change that.

@indykoning
Copy link
Copy Markdown
Member

image

@Jade-GG Jade-GG force-pushed the feature/auto-retry branch from f024ece to e745578 Compare April 8, 2026 11:31
@Jade-GG
Copy link
Copy Markdown
Collaborator Author

Jade-GG commented Apr 8, 2026

Error fixed by reverting the change, but now the screenshots are still slightly off with the font rendering.

@Jade-GG Jade-GG force-pushed the feature/auto-retry branch from c0e5a9b to ebbf244 Compare April 13, 2026 08:32
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

Successfully merging this pull request may close these issues.

2 participants