Files
rustfs/docs/fix-nosuchkey-regression.md
houseme 069194f553 Fix/getobjectlength (#920)
* fix getobject content length resp

* Fix regression in exception handling for non-existent key with enhanced compression predicate and metadata improvements (#915)

* Initial plan

* Fix GetObject regression by excluding error responses from compression

The issue was that CompressionLayer was attempting to compress error responses,
which could cause Content-Length header mismatches. By excluding 4xx and 5xx
responses from compression, we ensure error responses (like NoSuchKey) are sent
correctly without body truncation.

Co-authored-by: houseme <4829346+houseme@users.noreply.github.com>

* Enhance NoSuchKey fix with improved compression predicate and comprehensive tests

- Enhanced ShouldCompress predicate with size-based exclusion (< 256 bytes)
- Added detailed documentation explaining the compression logic
- Added debug logging for better observability
- Created comprehensive test suite with 4 test cases:
  - test_get_deleted_object_returns_nosuchkey
  - test_head_deleted_object_returns_nosuchkey
  - test_get_nonexistent_object_returns_nosuchkey
  - test_multiple_gets_deleted_object
- Added extensive inline documentation and comments
- Created docs/fix-nosuchkey-regression.md with full analysis

Co-authored-by: houseme <4829346+houseme@users.noreply.github.com>

* Add compression best practices documentation

Added comprehensive guide covering:
- Best practices for HTTP response compression
- Common pitfalls and solutions
- Performance considerations and trade-offs
- Testing guidelines and examples
- Monitoring and alerting recommendations
- Migration guide for existing services

Co-authored-by: houseme <4829346+houseme@users.noreply.github.com>

* fix

* fmt

* fmt

* Fix/objectdelete (#917)

* fix getobject content length resp

* fix delete object

---------

Co-authored-by: houseme <housemecn@gmail.com>

* Add comprehensive analysis of NoSuchKey fix and related improvements

Created detailed documentation analyzing:
- HTTP compression layer fix (primary issue)
- Content-length calculation fix from PR #917
- Delete object metadata fixes from PR #917
- How all components work together
- Complete scenario walkthrough
- Performance impact analysis
- Testing strategy and deployment checklist

This ties together all the changes in the PR branch including the merged
improvements from PR #917.

Co-authored-by: houseme <4829346+houseme@users.noreply.github.com>

* replace `once_cell` to `std`

* fmt

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: houseme <4829346+houseme@users.noreply.github.com>
Co-authored-by: houseme <housemecn@gmail.com>
Co-authored-by: weisd <im@weisd.in>

* fmt

---------

Co-authored-by: weisd <weishidavip@163.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: houseme <4829346+houseme@users.noreply.github.com>
Co-authored-by: weisd <im@weisd.in>
2025-11-24 18:56:34 +08:00

142 lines
5.2 KiB
Markdown

# Fix for NoSuchKey Error Response Regression (Issue #901)
## Problem Statement
In RustFS version 1.0.69, a regression was introduced where attempting to download a non-existent or deleted object would return a networking error instead of the expected `NoSuchKey` S3 error:
```
Expected: Aws::S3::Errors::NoSuchKey
Actual: Seahorse::Client::NetworkingError: "http response body truncated, expected 119 bytes, received 0 bytes"
```
## Root Cause Analysis
The issue was caused by the `CompressionLayer` middleware being applied to **all** HTTP responses, including S3 error responses. The sequence of events that led to the bug:
1. Client requests a non-existent object via `GetObject`
2. RustFS determines the object doesn't exist
3. The s3s library generates a `NoSuchKey` error response (XML format, ~119 bytes)
4. HTTP headers are written, including `Content-Length: 119`
5. The `CompressionLayer` attempts to compress the error response body
6. Due to compression buffering or encoding issues with small payloads, the body becomes empty (0 bytes)
7. The client receives `Content-Length: 119` but the actual body is 0 bytes
8. AWS SDK throws a "truncated body" networking error instead of parsing the S3 error
## Solution
The fix implements an intelligent compression predicate (`ShouldCompress`) that excludes certain responses from compression:
### Exclusion Criteria
1. **Error Responses (4xx and 5xx)**: Never compress error responses to ensure error details are preserved and transmitted accurately
2. **Small Responses (< 256 bytes)**: Skip compression for very small responses where compression overhead outweighs benefits
### Implementation Details
```rust
impl Predicate for ShouldCompress {
fn should_compress<B>(&self, response: &Response<B>) -> bool
where
B: http_body::Body,
{
let status = response.status();
// Never compress error responses (4xx and 5xx status codes)
if status.is_client_error() || status.is_server_error() {
debug!("Skipping compression for error response: status={}", status.as_u16());
return false;
}
// Check Content-Length header to avoid compressing very small responses
if let Some(content_length) = response.headers().get(http::header::CONTENT_LENGTH) {
if let Ok(length_str) = content_length.to_str() {
if let Ok(length) = length_str.parse::<u64>() {
if length < 256 {
debug!("Skipping compression for small response: size={} bytes", length);
return false;
}
}
}
}
// Compress successful responses with sufficient size
true
}
}
```
## Benefits
1. **Correctness**: Error responses are now transmitted with accurate Content-Length headers
2. **Compatibility**: AWS SDKs and other S3 clients correctly receive and parse error responses
3. **Performance**: Small responses avoid unnecessary compression overhead
4. **Observability**: Debug logging provides visibility into compression decisions
## Testing
Comprehensive test coverage was added to prevent future regressions:
### Test Cases
1. **`test_get_deleted_object_returns_nosuchkey`**: Verifies that getting a deleted object returns NoSuchKey
2. **`test_head_deleted_object_returns_nosuchkey`**: Verifies HeadObject also returns NoSuchKey for deleted objects
3. **`test_get_nonexistent_object_returns_nosuchkey`**: Tests objects that never existed
4. **`test_multiple_gets_deleted_object`**: Ensures stability across multiple consecutive requests
### Running Tests
```bash
# Run the specific test
cargo test --test get_deleted_object_test -- --ignored
# Or start RustFS server and run tests
./scripts/dev_rustfs.sh
cargo test --test get_deleted_object_test
```
## Impact Assessment
### Affected APIs
- `GetObject`
- `HeadObject`
- Any S3 API that returns 4xx/5xx error responses
### Backward Compatibility
- **No breaking changes**: The fix only affects error response handling
- **Improved compatibility**: Better alignment with S3 specification and AWS SDK expectations
- **No performance degradation**: Small responses were already not compressed by default in most cases
## Deployment Considerations
### Verification Steps
1. Deploy the fix to a staging environment
2. Run the provided Ruby reproduction script to verify the fix
3. Monitor error logs for any compression-related warnings
4. Verify that large successful responses are still being compressed
### Monitoring
Enable debug logging to observe compression decisions:
```bash
RUST_LOG=rustfs::server::http=debug
```
Look for log messages like:
- `Skipping compression for error response: status=404`
- `Skipping compression for small response: size=119 bytes`
## Related Issues
- Issue #901: Regression in exception when downloading non-existent key in alpha 69
- Commit: 86185703836c9584ba14b1b869e1e2c4598126e0 (getobjectlength fix)
## References
- [AWS S3 Error Responses](https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html)
- [tower-http CompressionLayer](https://docs.rs/tower-http/latest/tower_http/compression/index.html)
- [s3s Library](https://github.com/Nugine/s3s)