Recently I launched a client site that heavily used RedirectResponse
for add to cart -> checkout, checkout redirection, auto-checkout, and others. We experienced some issues, but really hit a bug when finally we kicked on the page cache expiration to be a day rather than zero.
One of the biggest culprits was bad / lacking cache metadata which caused bug reported in #2573807: Fatal error: Call to a member function getTimestamp().
The end result made things happy:
$url = Url::fromRoute('commerce_checkout.form', ['commerce_order' => $cart->id()])->toString(TRUE);
$response = new TrustedRedirectResponse($url->getGeneratedUrl());
$response->addCacheableDependency($url);
$response->addCacheableDependency($cart);
$event->setResponse($response);
So any time we throw a redirect we should pass $url->getGeneratedUrl()
and also then $response->addCacheableDependency($url);
ALSO, we should document the need for specifying cache contexts on URL parameters. I had to run the following which caused #2877498: Dynamic cache does not respect query parameters
$url = Url::fromRoute('commerce_checkout.form', ['commerce_order' => $order->id(), 'step' => $step_id])->toString(TRUE);
$response = new TrustedRedirectResponse($url->getGeneratedUrl());
$response->addCacheableDependency($url);
$response->addCacheableDependency($order);
$response->getCacheableMetadata()->addCacheContexts(['url.query_args:zuoraEc', ''url.query_args:refid']);
So maybe we'll need payment gateways to specify query params which bust cache? We had instances where changing the query param on the checkout form did not cause our redirect. And we can't just kill the entire cache for the checkout form, because performance.
Comments
Comment #2
Wim LeersPlease see #2877498-12: Dynamic cache does not respect query parameters.
Comment #3
Wim LeersThe main question here is: why do you want to use cacheable redirects? Is that appropriate here?
Because:
class TrustedRedirectResponse extends CacheableSecuredRedirectResponse
.Comment #4
skyredwangI ran into this problem in multiple projects. Here is a related core bug: #2744715: Generated URLs should only have cache contexts on GET/HEAD
Comment #5
mglamanHere is one reason why; out of habit:
We have to use TrustedRedirectResponse for external URLs, like off-site payment gateways (PayPal.) But I over used it for local redirects.
Comment #6
mglamanOkay, we're good in Commerce. Our main way of redirecting is the
NeedsRedirectException
which is a helper aroundEnforcedResponseException
andTrustedRedirectResponse
.Cache max-age set to 0. Checkout uses this, payment checkout panes use this.
All this hoopla was on my end not properly implementing redirects and having a frantic week.