# TODO: Batch Fetch Implementation Issues
## Critical Issues
### Time Handling
- [x] **Fix time precision loss in `system_time_to_offset_datetime`**
- ~~Current: `from_unix_timestamp(duration.as_secs() as i64)` loses nanoseconds~~
- ✅ Now using: `from_unix_timestamp_nanos(duration.as_nanos() as i128)`
- Location: `src/batch_fetch.rs:212`
### Missing Headers
- [x] **Extract and expose `Nats-Last-Sequence` header**
- Required by ADR-31 specification
- ✅ Header is preserved in RawMessage's headers field via `format_headers()`
- Location: `src/batch_fetch.rs:437` in `convert_to_raw_message`
## Error Handling
### Missing Status Code Handling
- [x] **Handle 408 status code for invalid requests**
- ADR-31 specifies 408 for bad/invalid requests
- ✅ Added handling with `BatchFetchErrorKind::InvalidRequest`
- Location: `src/batch_fetch.rs:442-445`
- [x] **Handle 413 status code for too many subjects**
- Returned when multi_last has >1024 subjects
- ✅ Added handling with `BatchFetchErrorKind::TooManySubjects`
- Location: `src/batch_fetch.rs:442-445`
## Validation
### Request Validation
- [x] **Add batch size validation**
- Server limit: 1000 messages maximum
- ✅ Validating in `GetBatchBuilder::send()` before sending request
- ✅ Test: `test_batch_size_limit`
- Location: `src/batch_fetch.rs:279`
- [x] **Add subject count validation for multi_last**
- Server limit: 1024 subjects maximum
- ✅ Validating in `GetLastBuilder::send()` before sending request
- ✅ Test: `test_subject_count_limit`
- Location: `src/batch_fetch.rs:341`
- [x] **Add stream name validation**
- ✅ Check for empty string in both builders
- ✅ Test: `test_invalid_stream_name`
- Location: Both builder `send()` methods
## Error Types
### Improve Error Specificity
- [x] **Add specific error kinds instead of using `Other`**
- ✅ Added `BatchFetchErrorKind::BatchSizeRequired` for missing batch size
- ✅ Added `BatchFetchErrorKind::SubjectsRequired` for missing subjects
- ✅ Added `BatchFetchErrorKind::BatchSizeTooLarge` for >1000 batch
- ✅ Added `BatchFetchErrorKind::TooManySubjects` for >1024 subjects
- ✅ Added `BatchFetchErrorKind::InvalidStreamName` for empty stream names
- ✅ Added `BatchFetchErrorKind::InvalidRequest` for 408 errors
- Location: `src/batch_fetch.rs:129-148`
## Feature Parity
### Stream Integration
- [ ] **Consider adding batch_fetch methods to Stream**
- async-nats Stream has direct_get methods
- Our implementation only extends Context
- Would provide better API consistency
### Single Message Operations
- [ ] **Consider adding single message direct_get support**
- Currently only batch operations
- Stream has `direct_get(seq)` for single message
- Could add `get_message(stream).seq(n).send()` builder
## Documentation
### API Documentation
- [x] **Document server limits in builder methods**
- ✅ Batch size limit (1000) documented
- ✅ Subject count limit (1024) documented
- ✅ Added to method documentation with `# Limits` sections
- [x] **Add more examples**
- ✅ Example with error handling: `examples/batch_fetch_with_error_handling.rs`
- ✅ Example with max_bytes limit: `examples/batch_fetch_with_max_bytes.rs`
- ✅ Example with time-based fetching: `examples/batch_fetch_time_based.rs`
## Testing
### Additional Test Coverage
- [ ] **Add test for 408 error handling**
- Test invalid request parameters (needs actual server support)
- [ ] **Add test for 413 error handling**
- Test with >1024 subjects (needs actual server support)
- [x] **Add test for batch size validation**
- ✅ Test: `test_batch_size_limit` - validates >1000 batch size fails
- [x] **Add test for time-based fetching**
- ✅ Test: `test_time_based_fetching` - verifies time precision is maintained
- [ ] **Add test for max_bytes limit**
- Verify server respects byte limits (needs actual server support)
## Performance
### Optimization Opportunities
- [ ] **Consider connection pooling for high-throughput scenarios**
- Multiple concurrent batch fetches could benefit
- [ ] **Add configurable buffer sizes for MessageStream**
- Currently using default async_stream buffer
## Priority Order
### ✅ Completed (Critical & Important)
1. **Critical**: Fix time precision loss ✅
2. **Critical**: Add Nats-Last-Sequence header extraction ✅
3. **Important**: Add 408/413 error handling ✅
4. **Important**: Add validation for batch/subject limits ✅
5. **Important**: Add stream name validation ✅
6. **Important**: Add specific error kinds ✅
### 📋 Remaining (Nice to have)
1. **Nice to have**: Stream integration
2. **Nice to have**: Single message operations
3. **Nice to have**: Additional test coverage (partially done)
4. **Nice to have**: Documentation improvements