Commit 365f29d
feat(core): integrate RetryManager into SegmentDestination upload pipeline (#1160)
* fix: add persistence validation, atomic backoff calc, and 429 precedence
- Validate persisted state in canRetry() to handle clock changes/corruption
per SDD §Metadata Lifecycle
- Move backoff calculation inside dispatch to avoid stale retryCount from
concurrent batch failures (handleErrorWithBackoff)
- Ensure RATE_LIMITED state is never downgraded to BACKING_OFF
- Update reset() docstring to clarify when it should be called
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: clean up RetryManager comments for post-merge readability
Simplify class docstring to describe current architecture without
referencing SDD deviations. Remove redundant inline comments, compact
JSDoc to single-line where appropriate, and ensure all comments use
present tense.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: consolidate error handlers, fix getState race, add limit signaling
- Use getState(true) for queue-safe reads to prevent race conditions
between concurrent canRetry/handle429/handleTransientError calls
- Consolidate handleError and handleErrorWithBackoff into a single
method that accepts a computeWaitUntilTime function
- Extract side effects (logging, Math.random) from dispatch reducers
- Return RetryResult ('rate_limited'|'backed_off'|'limit_exceeded')
from handle429/handleTransientError so callers can drop events on
limit exceeded
- Clear auto-flush timer in transitionToReady
- Validate state string in isPersistedStateValid
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* style: trim verbose inline comments in RetryManager
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: use enums and switch statements for clearer state handling
Replace string literals with TypeScript enums:
- RetryState enum (READY, RATE_LIMITED, BACKING_OFF)
- RetryResult enum (RATE_LIMITED, BACKED_OFF, LIMIT_EXCEEDED)
Extract helper methods for clarity:
- resolveStatePrecedence(): handles 429 taking priority over backoff
- consolidateWaitTime(): uses switch statement for clear wait time logic
- getStateDisplayName(): maps state to display names
Benefits:
- Type-safe state handling (no magic strings)
- Switch statements make control flow explicit
- Each helper method has a single, named responsibility
- Easier to test and maintain
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* refactor: remove VALID_STATES Set, use Object.values for enum validation
Use Object.values(RetryState).includes() instead of maintaining a
duplicate Set of valid states. More idiomatic TypeScript and eliminates
maintenance burden of keeping Set in sync with enum.
* test: add 429 authority test and fix autoFlush timer cleanup
- Add test verifying 429 Retry-After overrides long transient backoff
- Track RetryManager instances in autoFlush tests and destroy in
afterEach to prevent timer leaks
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* test: update tests for refactored API and add missing coverage
Update log assertions for consolidated limit-exceeded message. Add tests
for: RetryResult return values, jitter calculation, isPersistedStateValid
clock skew detection, handle429(0) edge case, and strengthened assertions
that verify behavioral state after clamping/rejection (not just log output).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: add persistence validation, atomic backoff calc, and 429 precedence
- Validate persisted state in canRetry() to handle clock changes/corruption
per SDD §Metadata Lifecycle
- Move backoff calculation inside dispatch to avoid stale retryCount from
concurrent batch failures (handleErrorWithBackoff)
- Ensure RATE_LIMITED state is never downgraded to BACKING_OFF
- Update reset() docstring to clarify when it should be called
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: clean up RetryManager comments for post-merge readability
Simplify class docstring to describe current architecture without
referencing SDD deviations. Remove redundant inline comments, compact
JSDoc to single-line where appropriate, and ensure all comments use
present tense.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: consolidate error handlers, fix getState race, add limit signaling
- Use getState(true) for queue-safe reads to prevent race conditions
between concurrent canRetry/handle429/handleTransientError calls
- Consolidate handleError and handleErrorWithBackoff into a single
method that accepts a computeWaitUntilTime function
- Extract side effects (logging, Math.random) from dispatch reducers
- Return RetryResult ('rate_limited'|'backed_off'|'limit_exceeded')
from handle429/handleTransientError so callers can drop events on
limit exceeded
- Clear auto-flush timer in transitionToReady
- Validate state string in isPersistedStateValid
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* style: trim verbose inline comments in RetryManager
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: use enums and switch statements for clearer state handling
Replace string literals with TypeScript enums:
- RetryState enum (READY, RATE_LIMITED, BACKING_OFF)
- RetryResult enum (RATE_LIMITED, BACKED_OFF, LIMIT_EXCEEDED)
Extract helper methods for clarity:
- resolveStatePrecedence(): handles 429 taking priority over backoff
- consolidateWaitTime(): uses switch statement for clear wait time logic
- getStateDisplayName(): maps state to display names
Benefits:
- Type-safe state handling (no magic strings)
- Switch statements make control flow explicit
- Each helper method has a single, named responsibility
- Easier to test and maintain
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* refactor: remove VALID_STATES Set, use Object.values for enum validation
Use Object.values(RetryState).includes() instead of maintaining a
duplicate Set of valid states. More idiomatic TypeScript and eliminates
maintenance burden of keeping Set in sync with enum.
* feat(core): integrate RetryManager into SegmentDestination upload pipeline
Wire RetryManager into SegmentDestination for TAPI-compliant retry
handling: uploadBatch() with error classification, event pruning on
partial failures, retry count header propagation, and QueueFlushingPlugin
error type handling updates.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: partial success reset, 429 precedence, and cleanup in sendEvents
- Don't reset retry state on partial success when concurrent batches have
429/transient errors
- Use if/else if so 429 takes precedence over transient error handling
- Remove redundant return Promise.resolve() in async function
- Fix duplicate keepalive property from master merge
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address review issues in SegmentDestination integration
- Use res.ok instead of res.status === 200 for 2xx success range
- Remove dead dequeue() method from QueueFlushingPlugin
- Add destroy() to SegmentDestination for RetryManager timer cleanup
- Reset retry state when queue is empty at flush time or after pruning
- Call handle429 per result instead of pre-aggregating, so
RetryManager.applyRetryStrategy respects eager/lazy consolidation
- Simplify retryAfterSeconds fallback to ?? 60
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: extract helper methods and clean up comments in SegmentDestination
Extract pruneExpiredEvents() and updateRetryState() from sendEvents to
improve readability. Remove redundant/obvious comments, merge duplicate
switch cases, and simplify return statements throughout.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: wire shutdown lifecycle, handle limit-exceeded signal, age-based pruning
- Override shutdown() instead of standalone destroy() to integrate with
the plugin lifecycle — prevents auto-flush timer leak on client cleanup
- Handle RetryResult 'limit_exceeded' from RetryManager: log warning and
let per-event age pruning (pruneExpiredEvents via _queuedAt) handle
event drops rather than dropping all retryable events on global counter
reset
- Import RetryResult type for type-safe limit checking
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: restore pre-existing comments in SegmentDestination.ts
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: always apply defaultHttpConfig, drop events on retry limit exceeded
Root cause: when the CDN settings response had no httpConfig field,
analytics.ts guarded the entire merge block with if (resJson.httpConfig),
so this.httpConfig stayed undefined. This prevented RetryManager creation
in SegmentDestination, disabling all retry features (error classification
overrides, canRetry() gating, retry counting, maxRetries enforcement).
Changes:
- Remove httpConfig guard in analytics.ts — always merge defaultHttpConfig
as baseline, with CDN and config.httpConfig overrides on top
- Add httpConfig?: DeepPartial<HttpConfig> to Config type for client-side
overrides (e.g. maxRetries from test harness)
- Drop retryable events when retry limits exceeded in SegmentDestination
instead of leaving them in the queue indefinitely
- Update fetchSettings test to expect defaultHttpConfig when CDN has no
httpConfig
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* style: trim verbose inline comments in SDK plugins and types
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: remove auto-flush wiring from SegmentDestination, add CDN validation tests
Remove autoFlushOnRetryReady check, setAutoFlushCallback call, and shutdown()
method. Add tests for CDN integrations validation (null, array, string, and
no-defaults scenarios).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: increment droppedEventCount at SegmentDestination drop sites
Wire up the droppedEventCount counter (added in cli-flush-retry-loop)
at the two places SegmentDestination permanently removes events:
permanent errors and retry limit exceeded.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: treat missing CDN integrations as empty, not as fallback trigger
When CDN returns a valid 200 with no integrations field (e.g. {}),
treat it as authoritative "no integrations configured" rather than
falling back to defaultSettings. This ensures SegmentDestination is
correctly disabled when the server has no integrations.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: restore droppedEventCount property on SegmentDestination
Property declaration was lost during branch rebase.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: replace droppedEventCount with reportInternalError at drop sites
Report EventsDropped errors via errorHandler at all three drop sites:
expired events, permanent HTTP errors, and retry limit exceeded.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: add metadata to EventsDropped errors with droppedCount and reason
Pass structured metadata at all three drop sites so consumers can
access droppedCount and reason without parsing strings.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: remove retryStrategy param, update tests for eager behavior
* refactor: improve SegmentDestination readability
- Extract error classification into classifyBatchResult method
- Add helper methods for config access (getRateLimitConfig, getBackoffConfig)
- Consolidate drop reporting into reportDroppedEvents helper
- Extract upload result processing into processUploadResults method
- Use early returns in sendEvents for clearer control flow
* fix: remove leftover activeManager reference from rebase
* chore: remove review-agent-commit.sh and add *.local.sh to gitignore
* fix: remove duplicate isPersistedStateValid calls in canRetry
* refactor: extract httpConfig merging into reusable helper
Consolidate duplicate 3-way config merging logic (default < CDN < client)
into mergeHttpConfig() helper in config-validation.ts. Reduces analytics.ts
by 42 lines and makes the merging logic testable and maintainable.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* fix: omit X-Retry-Count header on first request attempt
The X-Retry-Count header should only be sent on retry attempts (count > 0),
not on the initial request. This aligns with updated e2e test expectations
and standard retry header conventions.
Changes:
- api.ts: conditionally add X-Retry-Count only when retryCount > 0
- api.test.ts: update tests to verify header is omitted on first attempt
- e2e-config.json: enable retry test suite
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>1 parent aaf6348 commit 365f29d
14 files changed
Lines changed: 883 additions & 93 deletions
File tree
- e2e-cli
- packages/core/src
- __tests__
- internal
- backoff
- __tests__
- plugins
- __tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
97 | 97 | | |
98 | 98 | | |
99 | 99 | | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
100 | 103 | | |
101 | 104 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | | - | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
29 | 33 | | |
30 | 34 | | |
31 | 35 | | |
| |||
60 | 64 | | |
61 | 65 | | |
62 | 66 | | |
| 67 | + | |
63 | 68 | | |
64 | 69 | | |
65 | | - | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
66 | 80 | | |
67 | 81 | | |
68 | 82 | | |
| |||
74 | 88 | | |
75 | 89 | | |
76 | 90 | | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | 91 | | |
85 | 92 | | |
86 | 93 | | |
87 | 94 | | |
88 | 95 | | |
89 | 96 | | |
90 | | - | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
91 | 144 | | |
92 | 145 | | |
Lines changed: 87 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
| 2 | + | |
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| |||
436 | 436 | | |
437 | 437 | | |
438 | 438 | | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
439 | 513 | | |
440 | 514 | | |
441 | 515 | | |
| |||
483 | 557 | | |
484 | 558 | | |
485 | 559 | | |
486 | | - | |
| 560 | + | |
487 | 561 | | |
488 | 562 | | |
489 | 563 | | |
| |||
496 | 570 | | |
497 | 571 | | |
498 | 572 | | |
499 | | - | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
500 | 584 | | |
501 | 585 | | |
502 | 586 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
15 | 16 | | |
16 | 17 | | |
17 | 18 | | |
| |||
73 | 74 | | |
74 | 75 | | |
75 | 76 | | |
76 | | - | |
| 77 | + | |
77 | 78 | | |
78 | 79 | | |
79 | 80 | | |
| |||
198 | 199 | | |
199 | 200 | | |
200 | 201 | | |
201 | | - | |
202 | | - | |
| 202 | + | |
| 203 | + | |
203 | 204 | | |
204 | 205 | | |
205 | 206 | | |
| |||
418 | 419 | | |
419 | 420 | | |
420 | 421 | | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
421 | 428 | | |
422 | | - | |
423 | 429 | | |
424 | 430 | | |
425 | 431 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| 7 | + | |
7 | 8 | | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
| 12 | + | |
11 | 13 | | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
12 | 23 | | |
13 | 24 | | |
14 | 25 | | |
| |||
17 | 28 | | |
18 | 29 | | |
19 | 30 | | |
20 | | - | |
21 | | - | |
22 | | - | |
| 31 | + | |
23 | 32 | | |
24 | 33 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
126 | 126 | | |
127 | 127 | | |
128 | 128 | | |
129 | | - | |
130 | | - | |
131 | | - | |
132 | | - | |
133 | | - | |
134 | | - | |
135 | | - | |
136 | | - | |
137 | 129 | | |
138 | 130 | | |
139 | 131 | | |
| |||
0 commit comments