Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,29 @@ This gem supports the following versions of Ruby and Rails:
end
```

## Exception Handling

ResponseBank handles all exceptions gracefully during cache operations. If an exception occurs while reading from cache, deserializing cached data, or writing to cache, the middleware will:

1. **On cache read failures**: Fall back to rendering the page normally (as if it was a cache miss).
2. **On cache write failures**: Still serve the successfully rendered page to the user, but log the cache write failure.

This ensures that issues with cache stores (Redis/Memcached down), serialization errors, or compression/decompression failures don't cause 500 errors for your users.

### Custom Exception Handlers

You can set a custom exception handler in the Rack environment to be notified when cache operations fail (e.g., to report to Bugsnag, Sentry, etc.):

```ruby
# In an initializer or middleware
class MyMiddleware
def call(env)
env['response_bank.on_exception'] = ->(e) { Bugsnag.notify(e) }
@app.call(env)
end
end
```

## License

ResponseBank is released under the [MIT License](LICENSE.txt).
4 changes: 2 additions & 2 deletions lib/response_bank.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ class << self

DEFAULT_BROTLI_COMPRESSION_LEVEL = 7

DEFAULT_COMPRESSION_LEVEL = -> (_env, headers) {
case headers['Content-Encoding']
DEFAULT_COMPRESSION_LEVEL = -> (env, _headers) {
case env['response_bank.server_cache_encoding']
when 'br'
DEFAULT_BROTLI_COMPRESSION_LEVEL
when 'gzip'
Expand Down
80 changes: 46 additions & 34 deletions lib/response_bank/middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,43 +34,55 @@ def call(env)
body.each { |part| body_string << part }
end

body_compressed = nil
if body_string && body_string != ""
headers['Content-Encoding'] = content_encoding
env["cacheable.compression_level"] = ResponseBank.compression_level_for_request(env, headers)
time = ResponseBank.measure do
body_compressed = ResponseBank.compress(
body_string,
content_encoding,
compression_level: env["cacheable.compression_level"],
)
begin

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe this could go in a helper method now?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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)

body_compressed = nil
if body_string && body_string != ""
env["cacheable.compression_level"] = ResponseBank.compression_level_for_request(env, headers)
time = ResponseBank.measure do
body_compressed = ResponseBank.compress(
body_string,
content_encoding,
compression_level: env["cacheable.compression_level"],
)
end
ResponseBank.log("Compression time: #{time}ms")
env["cacheable.compression_time"] = time
headers['Content-Encoding'] = content_encoding
end
ResponseBank.log("Compression time: #{time}ms")
env["cacheable.compression_time"] = time
end

cached_headers = headers.slice(*CACHEABLE_HEADERS)
# Store result
cache_data = [status, cached_headers, body_compressed, timestamp, env["cacheable.compression_level"]]

ResponseBank.write_to_cache(env['cacheable.key']) do
payload = MessagePack.dump(cache_data)
ResponseBank.write_to_backing_cache_store(
env,
env['cacheable.unversioned-key'],
payload,
expires_in: env['cacheable.versioned-cache-expiry'],
)
end
cached_headers = headers.slice(*CACHEABLE_HEADERS)
# Store result
cache_data = [status, cached_headers, body_compressed, timestamp, env["cacheable.compression_level"]]

ResponseBank.write_to_cache(env['cacheable.key']) do
payload = MessagePack.dump(cache_data)
ResponseBank.write_to_backing_cache_store(
env,
env['cacheable.unversioned-key'],
payload,
expires_in: env['cacheable.versioned-cache-expiry'],
)
end

# since we had to generate the compressed version already we may
# as well serve it if the client wants it
if body_compressed
if env['HTTP_ACCEPT_ENCODING'].to_s.include?(content_encoding)
body = [body_compressed]
else
# Remove content-encoding header for response with compressed content
headers.delete('Content-Encoding')
# since we had to generate the compressed version already we may
# as well serve it if the client wants it
if body_compressed
if env['HTTP_ACCEPT_ENCODING'].to_s.include?(content_encoding)
body = [body_compressed]
else
# Remove content-encoding header for response with compressed content
headers.delete('Content-Encoding')
end
end
rescue => exception

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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).

headers.delete('Content-Encoding') if body_compressed
ResponseBank.log("Failed to write to cache: #{exception.class} - #{exception.message}")
if env['response_bank.on_exception']
begin
env['response_bank.on_exception'].call(exception)
rescue => handler_exception
ResponseBank.log("Exception handler failed: #{handler_exception.class} - #{handler_exception.message}")
end
end
end
end
Expand Down
31 changes: 25 additions & 6 deletions lib/response_bank/response_cache_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,25 @@ def cacheable_info_dump
end

def try_to_serve_from_cache
response = read_from_cache
return response if response

# No cache hit; this request cannot be handled from cache.
# Yield to the controller and mark for writing into cache.
refill_cache
end

def read_from_cache
# Etag
unless @skip_browser_cache
response = serve_from_browser_cache(entity_tag_hash, @env['HTTP_IF_NONE_MATCH'])
return response if response
end

response = serve_from_cache(cache_key_hash, @serve_unversioned ? "*" : entity_tag_hash, @cache_age_tolerance)
return response if response

# No cache hit; this request cannot be handled from cache.
# Yield to the controller and mark for writing into cache.
refill_cache
serve_from_cache(cache_key_hash, @serve_unversioned ? "*" : entity_tag_hash, @cache_age_tolerance)
rescue => exception
handle_cache_exception(exception)
nil
end

def serve_from_browser_cache(entity_tag, if_none_match)
Expand Down Expand Up @@ -204,5 +211,17 @@ def refill_cache

@cache_miss_block.call
end

def handle_cache_exception(exception)
ResponseBank.log("Cache operation failed: #{exception.class} - #{exception.message}")

if @env['response_bank.on_exception']
begin
@env['response_bank.on_exception'].call(exception)
rescue => handler_exception
ResponseBank.log("Exception handler failed: #{handler_exception.class} - #{handler_exception.message}")
end
end
end
end
end
77 changes: 77 additions & 0 deletions test/middleware_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -430,4 +430,81 @@ def test_cache_hit_client
assert_equal('client', @env['cacheable.store'])
assert_equal('"etag_value"', headers['ETag'])
end

def test_exception_during_compression_does_not_fail_request
ResponseBank.expects(:compress).raises(StandardError.new("Compression failed"))
ResponseBank.stubs(:log)
ResponseBank.expects(:log).with(includes("Failed to write to cache")).once

ware = ResponseBank::Middleware.new(method(:cacheable_app))
result = ware.call(@env)

# Should still return 200 OK with uncompressed body and no Content-Encoding
assert_equal(200, result[0])
assert_nil(result[1]['Content-Encoding'])
assert_equal('Hi', result[2].join)
end

def test_exception_during_serialization_does_not_fail_request
ResponseBank.expects(:compress).returns("compressed")
MessagePack.expects(:dump).raises(StandardError.new("Serialization failed"))
ResponseBank.stubs(:log)
ResponseBank.expects(:log).with(includes("Failed to write to cache")).once

ware = ResponseBank::Middleware.new(method(:cacheable_app))
result = ware.call(@env)

# Should still return 200 OK with uncompressed body and no Content-Encoding
assert_equal(200, result[0])
assert_nil(result[1]['Content-Encoding'])
assert_equal('Hi', result[2].join)
end

def test_exception_during_cache_write_does_not_fail_request
ResponseBank.cache_store.expects(:write).raises(StandardError.new("Cache store is down"))
ResponseBank.stubs(:log)
ResponseBank.expects(:log).with(includes("Failed to write to cache")).once

ware = ResponseBank::Middleware.new(method(:cacheable_app))
result = ware.call(@env)

# Should still return 200 OK
assert_equal(200, result[0])
assert_equal('Hi', result[2].join)
end

def test_custom_exception_handler_called_on_middleware_failure
exception_handled = false
exception_message = nil

@env['response_bank.on_exception'] = ->(e) {
exception_handled = true
exception_message = e.message
}

ResponseBank.expects(:compress).raises(StandardError.new("Compression failed"))
ResponseBank.stubs(:log)

ware = ResponseBank::Middleware.new(method(:cacheable_app))
result = ware.call(@env)

assert(exception_handled, "Custom exception handler should have been called")
assert_equal("Compression failed", exception_message)
assert_equal(200, result[0])
end

def test_exception_in_middleware_custom_handler_is_caught
@env['response_bank.on_exception'] = ->(e) { raise "Handler itself failed" }

ResponseBank.expects(:compress).raises(StandardError.new("Compression failed"))
ResponseBank.stubs(:log)
ResponseBank.expects(:log).with(includes("Exception handler failed")).once

ware = ResponseBank::Middleware.new(method(:cacheable_app))
result = ware.call(@env)

# Should still return 200 OK even if exception handler fails
assert_equal(200, result[0])
assert_equal('Hi', result[2].join)
end
end
61 changes: 61 additions & 0 deletions test/response_cache_handler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -298,4 +298,65 @@ def expect_page_rendered(cache_entry, content_encoding = 'br')

body
end

def test_exception_during_cache_read_falls_back_to_refill
@cache_store.expects(:read).raises(StandardError.new("Cache store is down"))
ResponseBank.stubs(:log)
ResponseBank.expects(:log).with(includes("Cache operation failed")).once

_, _, body = handler.run!
assert_equal('dynamic output', body)
assert_cache_miss(true, nil)
end

def test_exception_during_deserialization_falls_back_to_refill
@cache_store.expects(:read).returns("invalid msgpack data")
MessagePack.expects(:load).raises(MessagePack::MalformedFormatError.new("invalid data"))
ResponseBank.stubs(:log)
ResponseBank.expects(:log).with(includes("Cache operation failed")).once

_, _, body = handler.run!
assert_equal('dynamic output', body)
assert_cache_miss(true, nil)
end

def test_exception_during_decompression_falls_back_to_refill
@cache_store.expects(:read).returns(page_cache_entry(true, 'br'))
controller.request.env['HTTP_ACCEPT_ENCODING'] = 'gzip'
ResponseBank.expects(:decompress).raises(Brotli::Error.new("decompression failed"))
ResponseBank.stubs(:log)
ResponseBank.expects(:log).with(includes("Cache operation failed")).once

_, _, body = handler.run!
assert_equal('dynamic output', body)
assert_cache_miss(true, :anything)
end

def test_custom_exception_handler_is_called
exception_handled = false
exception_message = nil

controller.request.env['response_bank.on_exception'] = ->(e) {
exception_handled = true
exception_message = e.message
}

@cache_store.expects(:read).raises(StandardError.new("Cache store is down"))
ResponseBank.stubs(:log)
handler.run!

assert(exception_handled, "Custom exception handler should have been called")
assert_equal("Cache store is down", exception_message)
end

def test_exception_in_custom_handler_is_caught
controller.request.env['response_bank.on_exception'] = ->(e) { raise "Handler itself failed" }

@cache_store.expects(:read).raises(StandardError.new("Cache store is down"))
ResponseBank.stubs(:log)
ResponseBank.expects(:log).with(includes("Exception handler failed")).once

_, _, body = handler.run!
assert_equal('dynamic output', body)
end
end