Error pages other than 404s are uncacheable
jpqy opened this issue · 1 comments
Currently, response_bank
only caches pages if the response code is a 200, 301, or 404:
This prevents response_bank
from being used to cache error pages other than 404s, even though caching other error pages (such as 429s) would bring the same performance benefits as caching 404 pages.
For example, only the 404 error page would be cached in this example:
class ErrorsController < ApplicationController
def not_found
response_cache(error_code: 404) do
render(template: "errors/not_found", status: :not_found)
end
end
def too_many_requests
response_cache(error_code: 429) do
render(template: "errors/too_many_requests", status: :too_many_requests)
end
end
def internal_server_error
response_cache(error_code: 500) do
render(template: "errors/internal_server_error", status: :internal_server_error)
end
end
end
Caching responses with other error status codes that may depend on the state of the client (e.g. 429s depend on how many times the client has already hit the server) can be tricky, so I would suggest allowing response_bank
to accept a configuration option that overrides the default array of cacheable response codes.
Please see https://github.com/Shopify/shopify-app-store/issues/17194 and https://github.com/Shopify/shopify-app-store/pull/17365/files#r901784102 and for more context on why App Store and Theme Store could use this change.
Please feel free to submit a pull request.