Open
Conversation
ChrisBr
reviewed
Feb 6, 2026
| content_encoding, | ||
| compression_level: env["cacheable.compression_level"], | ||
| ) | ||
| begin |
There was a problem hiding this comment.
Maybe this could go in a helper method now?
Author
There was a problem hiding this comment.
Trying it out I would say it makes the code worse.
The begin/rescue in call was already clear. Now you have a compress_and_cache_response method that shadows ResponseBank.write_to_cache, takes 5 positional arguments, and mutates headers and env as side effects. It hasn't really reduced complexity — it's just moved code around.
def compress_and_cache_response(body_string, status, headers, content_encoding, env)
Extract read_from_cache from try_to_serve_from_cache and wrap it in a rescue block so that exceptions during cache reads, deserialization, or decompression fall back to a normal cache miss instead of raising a 500. Add handle_cache_exception helper with support for a custom response_bank.on_exception callback (e.g. for error reporting to Bugsnag/Sentry).
andheiberg
force-pushed
the
rescue-aggresively
branch
from
3c9376f to
e47ade5
Compare
Wrap the compression, serialization, and cache write path in the middleware with a begin/rescue so that failures don't cause 500 errors. On exception the Content-Encoding header is stripped and the original uncompressed response is served. The response_bank.on_exception callback is invoked if set.
andheiberg
force-pushed
the
rescue-aggresively
branch
2 times, most recently
from
bdebbb3 to
8ec2378
Compare
andheiberg
force-pushed
the
rescue-aggresively
branch
from
8ec2378 to
504fa7c
Compare
ChrisBr
approved these changes
Feb 18, 2026
etiennebarrie
approved these changes
Feb 18, 2026
| headers.delete('Content-Encoding') | ||
| end | ||
| end | ||
| rescue => exception |
Member
There was a problem hiding this comment.
Some exceptions (those not inheriting from StandardError) might still slip through, but I think it's OK because these exceptions should only be used when it's not recoverable (e.g. SyntaxError, NoMemoryError).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Should an exception be raised while serisalising, compressing, writing to cache, fetching from cache, decompressing or deserialising we would previously raise that exception and return an error.
With this PR the exception will be caught and the app run as if the response_cache gem was disabled.