Skip to content

feat(core): add TAPI error classification types and utilities#1156

Merged
abueide merged 12 commits intomasterfrom
tapi/types-and-errors
Mar 19, 2026
Merged

feat(core): add TAPI error classification types and utilities#1156
abueide merged 12 commits intomasterfrom
tapi/types-and-errors

Conversation

@abueide
Copy link
Contributor

@abueide abueide commented Mar 10, 2026

Summary

  • Add foundation types: HttpConfig, BackoffConfig, RateLimitConfig, ErrorClassification, and related config fields to Config and SegmentAPISettings
  • Add error classification utilities: classifyError(), parseRetryAfter(), and improved translateHTTPError()
  • Add comprehensive test suite (35 tests) covering status code classification, retry-after parsing, and SDD-specified overrides
  • Fix parseRetryAfter to use strict /^\d+$/ regex instead of parseInt, preventing date strings starting with digits from being misclassified as seconds
  • Add test for 429 + statusCodeOverride='drop' precedence behavior
  • Add test for date-string-starting-with-digits edge case
  • Fix formatting in e2e-cli files to pass CI format-check

PR 1 of 5 in the TAPI backoff/retry stack. See #1154 for full context.

Test plan

  • All 35 error classification tests pass
  • parseRetryAfter("01 Jan 2026 00:01:00 GMT") correctly parses as date (60s), not as integer (1s)
  • classifyError(429, { statusCodeOverrides: { '429': 'drop' } }) returns permanent
  • CI format-check passes

🤖 Generated with Claude Code

@abueide abueide force-pushed the tapi/types-and-errors branch 2 times, most recently from eedad38 to 36997ac Compare March 12, 2026 16:11
* When true, automatically triggers a flush when the retry manager's wait
* period expires and transitions back to READY. Disabled by default.
*/
autoFlushOnRetryReady?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

likewise. Part of me wants this to go in every SDK, but I think existing flush policy with timers is enough control for users.

Open to debating it tho

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can agree I don't have any qualms with removing it.

@abueide abueide force-pushed the tapi/types-and-errors branch 3 times, most recently from b1cd5f5 to ff837e7 Compare March 19, 2026 16:15
abueide and others added 11 commits March 19, 2026 11:54
Add foundation types (HttpConfig, BackoffConfig, RateLimitConfig,
ErrorClassification) and error classification utilities (classifyError,
parseRetryAfter, translateHTTPError improvements) for TAPI-compliant
retry handling.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remnants from the removed BackoffManager and UploadStateMachine classes,
never imported anywhere.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Convert ErrorClassification from a plain type to a class with a getter,
eliminating redundant isRetryable field that was always derivable from
errorType !== 'permanent'.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
parseRetryAfter now returns undefined for negative seconds instead
of falling through to date parsing where the behavior was ambiguous.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…verage

Use regex /^\d+$/ instead of parseInt to prevent date strings starting
with digits from being misclassified as seconds. Add test for
429+override='drop' precedence and date-string edge case. Run formatter
to fix CI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Restore JSDoc on ErrorType enum and inline comments in
translateHTTPError that were inadvertently removed during cleanup.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Redundant with TimerFlushPolicy which already drives periodic flushes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@abueide abueide force-pushed the tapi/types-and-errors branch from ff837e7 to a3ac0e5 Compare March 19, 2026 16:57
- Remove retryStrategy config option from Config type (hardcoded to
  eager in RetryManager downstream)
- Add 14 SDD-default config tests verifying all spec status codes
  (408/410/460=retry, 501/505=drop, 429=rate_limit, etc.)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@abueide abueide force-pushed the tapi/types-and-errors branch from ec82caa to 6b03226 Compare March 19, 2026 17:55
@abueide abueide merged commit 3f0234e into master Mar 19, 2026
7 checks passed
@abueide abueide deleted the tapi/types-and-errors branch March 19, 2026 18:17
abueide added a commit that referenced this pull request Mar 23, 2026
retryStrategy was removed from Config type in #1156, but remained in
defaultConfig causing build errors.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
abueide added a commit that referenced this pull request Mar 23, 2026
#1157)

* feat(core): add config validation, defaults, and httpConfig extraction

Add config validation utilities (validateRateLimitConfig,
validateBackoffConfig), default HTTP config constants, and httpConfig
extraction/merging logic in SegmentClient.fetchSettings().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: deep-merge statusCodeOverrides and add cross-config validation

Deep-merge statusCodeOverrides so server-sent partial overrides don't
replace defaults. Add rateLimitConfig parameter to validateBackoffConfig
for the cross-config relational constraint:
maxTotalBackoffDuration >= 2x max(maxBackoffInterval, maxRetryInterval).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: remove autoFlushOnRetryReady from defaults, add CDN integrations validation

Remove autoFlushOnRetryReady from defaultConfig (redundant with TimerFlushPolicy).
Validate integrations field from CDN response before storing — falls back to
defaultSettings when CDN returns null, array, or non-object integrations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: remove retryStrategy from defaultConfig

retryStrategy was removed from Config type in #1156, but remained in
defaultConfig causing build errors.

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>

* refactor: simplify fetchSettings by extracting helper methods

Extract complex logic into focused helper methods:
- validateIntegrations(): handles normalization and validation
- extractHttpConfig(): handles merge and validation of httpConfig
- applyDefaultSettings(): consolidates fallback logic

This reduces fetchSettings from 98 lines to ~50 lines and improves
testability by isolating each concern.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* refactor: move config helpers to config-validation module

Move validateIntegrations and extractHttpConfig from SegmentClient
private methods to exported functions in config-validation.ts.

This keeps analytics.ts slimmer and groups all config-related
validation logic in one place.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* style: format imports in analytics.ts

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants