feat: implement TAPI retry handling - Phase 2 (Settings Integration)#295
feat: implement TAPI retry handling - Phase 2 (Settings Integration)#295didiergarcia merged 4 commits intomainfrom
Conversation
Task 9: Add httpConfig field to SegmentSettings - Added httpConfig: JsonObject? field to SegmentSettings data class - Imported kotlinx.serialization.json.JsonObject Task 10: Create RetryConfigParser with defensive parsing - Parse httpConfig JSON from CDN into RetryConfig - Comprehensive defensive error handling: * Returns defaults on null/corrupt JSON * Clamps all numeric values to safe ranges * Validates retry behaviors and status codes * Never throws exceptions - 17 tests covering: * Default handling * Value clamping * Status code overrides * Error recovery * Partial configurations Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR implements Phase 2 of the TAPI retry handling system, adding the ability to parse retry configuration from CDN settings JSON. Building on Phase 1 (#293) which implemented the core state machine, this phase focuses solely on robust parsing with comprehensive defensive error handling. The implementation follows a "never throw" philosophy with extensive value clamping and validation.
Changes:
- Added
httpConfig: JsonObject?field toSegmentSettingsfor receiving retry configuration from CDN - Created
RetryConfigParserwith defensive parsing that clamps all numeric values to safe ranges and validates status codes and retry behaviors - Added 17 comprehensive tests covering default handling, value clamping, partial configurations, and corrupt JSON recovery
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| core/src/main/java/com/segment/analytics/kotlin/core/platform/plugins/SegmentDestination.kt | Added nullable httpConfig field to SegmentSettings for CDN retry configuration |
| core/src/main/java/com/segment/analytics/kotlin/core/retry/RetryConfigParser.kt | Implemented defensive parser with value clamping, validation, and fallback to defaults on errors |
| core/src/test/kotlin/com/segment/analytics/kotlin/core/retry/RetryConfigParserTest.kt | Added 17 tests covering all parser scenarios including edge cases and error conditions |
Comments suppressed due to low confidence (1)
core/src/main/java/com/segment/analytics/kotlin/core/retry/RetryConfigParser.kt:125
- When
statusCodeOverridesis provided as an empty JSON object{}, the parser returns an empty map, losing all default status code overrides (408, 410, 429, 460, 501, 505). This differs from the behavior of other fields where missing values fall back to defaults.
If the intent is to allow the CDN to completely clear all overrides by sending an empty object, this is fine but should be documented. Otherwise, consider whether an empty statusCodeOverrides should preserve defaults, similar to how a missing maxRetryCount field still uses the default value of 100.
private fun parseStatusCodeOverrides(json: JsonObject?): Map<Int, RetryBehavior> {
if (json == null) return BackoffConfig().statusCodeOverrides
return try {
json.mapNotNull { (key, value) ->
val statusCode = key.toIntOrNull()?.takeIf { it in 100..599 }
val behavior = parseRetryBehavior((value as? JsonPrimitive)?.contentOrNull, RetryBehavior.DROP)
if (statusCode != null) statusCode to behavior else null
}.toMap()
} catch (e: Exception) {
BackoffConfig().statusCodeOverrides
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Document that an empty statusCodeOverrides JSON object {} intentionally
clears all defaults, allowing the CDN to remove overrides when needed.
This differs from other fields but is the desired behavior.
Addresses low-confidence copilot review comment.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
core/src/main/java/com/segment/analytics/kotlin/core/platform/plugins/SegmentDestination.kt
Outdated
Show resolved
Hide resolved
core/src/main/java/com/segment/analytics/kotlin/core/retry/RetryConfigParser.kt
Outdated
Show resolved
Hide resolved
core/src/main/java/com/segment/analytics/kotlin/core/retry/RetryConfigParser.kt
Outdated
Show resolved
Hide resolved
core/src/main/java/com/segment/analytics/kotlin/core/retry/RetryConfigParser.kt
Outdated
Show resolved
Hide resolved
core/src/main/java/com/segment/analytics/kotlin/core/retry/RetryConfigParser.kt
Outdated
Show resolved
Hide resolved
…ateLimitConfig Renamed for clarity since 'backoff duration' semantically belongs to exponential backoff (5xx), not rate limiting (429). - RateLimitConfig.maxTotalBackoffDuration → maxRateLimitDuration - Updated parser to use new field name - All tests pass Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…stom serializer Address PR review feedback to use typed data class instead of manual JSON parsing. Changes: - Created HttpConfig data class with custom serializer - Added validation methods to RateLimitConfig and BackoffConfig - Updated SegmentSettings to use HttpConfig instead of JsonObject - Deleted RetryConfigParser (manual parsing no longer needed) - Created HttpConfigTest with 16 tests (covers validation and error handling) - Deleted RetryConfigParserTest (replaced by HttpConfigTest) Benefits: - Type-safe access to config properties (httpConfig.rateLimitConfig.enabled) - Automatic deserialization via kotlinx.serialization - Still provides defensive error handling via custom serializer - Maintains value clamping and validation - Cleaner code following codebase patterns Custom serializer ensures: - Corrupt JSON → returns default HttpConfig() - Out-of-range values → automatically clamped - Never throws exceptions All tests pass (16 new validation tests + 24 existing state machine tests) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary
Implements Phase 2 of TAPI retry handling system - Settings Integration. Adds the ability to parse retry configuration from CDN settings JSON.
Builds on Phase 1 (#293) which implemented the core state machine.
What's Included
Task 9: SegmentSettings Update
httpConfig: JsonObject?field toSegmentSettingsdata classTask 10: RetryConfigParser with Defensive Parsing
Created a robust parser that converts CDN JSON into
RetryConfigwith comprehensive error handling.Key Features:
Clamping Examples:
maxRetryInterval: 1s - 1 hour (clamped from wider range)baseBackoffInterval: 0.1s - 60smaxRetryCount: 0 - 1000jitterPercent: 0% - 50%maxTotalBackoffDuration: 0 - 1 weekTest Coverage
17 new tests covering:
All tests pass. No test regressions.
Files Changed
What's NOT Included
This PR intentionally excludes integration work:
The parser can be reviewed independently before integration.
Example Usage
Next Steps
After review and merge:
Testing
All 17 tests pass.
🤖 Generated with Claude Code