feat: add brotli and zstd decompression support for toolbar injection#37
feat: add brotli and zstd decompression support for toolbar injection#37JacobCoffee merged 1 commit intomainfrom
Conversation
Extends compression handling to support all compression types used by Litestar's compression middleware: - gzip (existing) - brotli (br) - optional, requires brotli package - zstd - optional, requires zstandard package Gracefully skips toolbar injection when compression libraries are unavailable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Pull request overview
This PR extends the debug toolbar's compression support to handle brotli and zstd encoding in addition to the existing gzip support, aligning with Litestar's compression middleware capabilities.
Key Changes:
- Added brotli (br) decompression support with graceful ImportError handling
- Added zstd decompression support with graceful ImportError handling
- Updated documentation and logging messages to reflect support for multiple compression formats
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except ImportError: | ||
| logger.debug("zstandard not installed, skipping toolbar injection for zstd-encoded response") | ||
| return body, content_encoding | ||
| except Exception: |
There was a problem hiding this comment.
The broad exception handler catches all exceptions from zstandard decompression. This can mask legitimate programming errors or unexpected issues. Consider catching specific exceptions like zstandard.ZstdError to handle only expected decompression failures, while allowing unexpected errors to propagate appropriately.
| except Exception: | |
| except zstandard.ZstdError: |
| try: | ||
| import brotli # type: ignore[import-untyped] | ||
|
|
||
| body = brotli.decompress(body) | ||
| decompressed = True | ||
| except ImportError: | ||
| logger.debug("Brotli not installed, skipping toolbar injection for br-encoded response") | ||
| return body, content_encoding | ||
| except Exception: | ||
| logger.debug("Invalid brotli data, attempting to decode as-is") | ||
|
|
||
| elif "zstd" in encodings: | ||
| try: | ||
| import zstandard # type: ignore[import-untyped] | ||
|
|
||
| dctx = zstandard.ZstdDecompressor() | ||
| body = dctx.decompress(body) | ||
| decompressed = True |
There was a problem hiding this comment.
The brotli and zstandard libraries are imported inline within a try-except block on every request that uses these compression methods. Consider importing these at the module level with a try-except to set flags (e.g., HAS_BROTLI, HAS_ZSTANDARD), then check those flags before attempting decompression. This would improve performance by avoiding repeated import attempts on every request and make the availability check more efficient.
| elif "br" in encodings: | ||
| try: | ||
| import brotli # type: ignore[import-untyped] | ||
|
|
||
| body = brotli.decompress(body) | ||
| decompressed = True | ||
| except ImportError: | ||
| logger.debug("Brotli not installed, skipping toolbar injection for br-encoded response") | ||
| return body, content_encoding | ||
| except Exception: | ||
| logger.debug("Invalid brotli data, attempting to decode as-is") |
There was a problem hiding this comment.
The new brotli decompression support lacks test coverage. The existing TestGzipCompression class demonstrates comprehensive testing patterns (valid compression, invalid data, UTF-8 decoding failures, case-insensitive headers) that should be replicated for brotli. Consider adding similar test cases for br-encoded responses to ensure the decompression logic works correctly and handles edge cases appropriately.
| elif "zstd" in encodings: | ||
| try: | ||
| import zstandard # type: ignore[import-untyped] | ||
|
|
||
| dctx = zstandard.ZstdDecompressor() | ||
| body = dctx.decompress(body) | ||
| decompressed = True | ||
| except ImportError: | ||
| logger.debug("zstandard not installed, skipping toolbar injection for zstd-encoded response") | ||
| return body, content_encoding | ||
| except Exception: | ||
| logger.debug("Invalid zstd data, attempting to decode as-is") |
There was a problem hiding this comment.
The new zstd decompression support lacks test coverage. The existing TestGzipCompression class demonstrates comprehensive testing patterns (valid compression, invalid data, UTF-8 decoding failures, case-insensitive headers) that should be replicated for zstd. Consider adding similar test cases for zstd-encoded responses to ensure the decompression logic works correctly and handles edge cases appropriately.
| except ImportError: | ||
| logger.debug("Brotli not installed, skipping toolbar injection for br-encoded response") | ||
| return body, content_encoding | ||
| except Exception: |
There was a problem hiding this comment.
The broad exception handler catches all exceptions from brotli decompression. This can mask legitimate programming errors or unexpected issues. Consider catching specific exceptions like brotli.error (if available) to handle only expected decompression failures, while allowing unexpected errors to propagate appropriately.
| except Exception: | |
| except brotli.error: |
1 participant
Summary
Related: litestar-org/litestar#4286
Test plan
make ci- tests pass, pre-existing type issues unrelated)🤖 Generated with Claude Code