feat(audience): HttpTransport — background POST with gzip + retry (SDK-141)#691
Merged
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f66dff6. Configure here.
f66dff6 to
b9faf5a
Compare
Gzip: compresses batch payloads using GZipStream (System.IO.Compression,
available in Unity 2021+ .NET Standard 2.1). Pure C#, all platforms.
HttpTransport: reads batches from DiskStore, wraps in {"batch":[...]},
gzips, POSTs to /v1/audience/messages with x-immutable-publishable-key
header. Derives sandbox vs production URL from key prefix.
Retry policy:
- 200: delete batch from disk
- 4xx: delete batch (validation error, won't succeed on retry)
- 5xx: keep on disk, exponential backoff (5s → 10s → 20s → 40s → 60s cap)
- Network error: same as 5xx
- Backoff resets after success
Testable via injected HttpMessageHandler — 14 tests, no network calls.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Plan specifies: 5s → 10s → 20s → 60s cap. Implementation had: 5s → 10s → 20s → 40s → 60s cap. Replace the bitshift formula with an explicit switch expression so the schedule is readable and matches the plan exactly. Update the test to verify the jump from 20s directly to 60s. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Match the pattern used elsewhere in the codebase (Session.cs uses 60_000 for heartbeat interval). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
b9faf5a to
52381a4
Compare
nattb8
reviewed
Apr 17, 2026
nattb8
reviewed
Apr 17, 2026
nattb8
reviewed
Apr 17, 2026
Replace the binary IsBackingOff flag with a time-aware window derived from NextAttemptAt. IsBackingOff returned true as long as any failure had occurred, which would cause a caller that skips-when-backing-off to skip forever after the first failure. Changes: - Add NextAttemptAt (DateTime?) — set on failure to now + BackoffMs, cleared on success. Null when there is no active backoff. - Replace IsBackingOff with IsInBackoffWindow, which compares UtcNow (via injectable clock) against NextAttemptAt. Naturally expires. - Inject clock via constructor for deterministic timing tests — falls back to DateTime.UtcNow when omitted. - Factor failure/success transitions into RecordFailure/ResetBackoff helpers so the two state fields stay consistent. - Rename IsBackingOff to IsInBackoffWindow in tests; add a timing test that advances the injected clock to verify the window closes at NextAttemptAt. Not fixup into the HttpTransport commit — kept separate so the scope of this design change is reviewable on its own and revertable if the simpler binary flag is preferred. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses review feedback on PR #691. The previous catch (Exception) in BuildPayload swallowed every failure as if it were the expected file-disappeared race. Permissions errors (UnauthorizedAccessException) and similar non-IO faults were silently dropped, masking real problems. Narrowing to IOException keeps the file-disappeared / locked / path- gone cases skipped — which is what the retry loop needs — while letting genuinely broken states propagate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses review feedback on PR #691. Makes the nullable return value explicit in the XML doc — the reviewer asked for Passport-style clarity on what's nullable. Enabling nullable reference types package-wide is a larger follow-up; this commit covers the specific case flagged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses nattb8's review on PR #691 plus follow-up cleanups. - Adopt Passport's per-file `#nullable enable` pattern. BuildPayload's return type becomes `string?`; optional ctor params (`onError`, `handler`, `getUtcNow`) and the `_onError` field are nullable-annotated. - Restore the 40s step in the backoff schedule (5s → 10s → 20s → 40s → 60s cap) so doubling stays clean and matches the original bitshift formula's natural shape. - Rewrite HttpTransport.cs comments as direct, plain-English facts. Class summary trimmed to a one-line role description; per-branch intent annotated only where non-obvious (4xx reset-backoff rationale, cancellation `when` guard, IOException narrow catch, NotifyError swallow). Drop the redundant BackoffMs summary that duplicated the switch arms below it. - Fix the cancellation-catch comment that incorrectly attributed token tripping to HttpTransport.Dispose (it's the caller's responsibility). - Handle non-IOException storage failures in SendBatchAsync. BuildPayload narrows its catch to IOException; everything else (e.g. UnauthorizedAccessException from stripped permissions) used to escape SendBatchAsync entirely and would silently kill the flush loop. Wrap the BuildPayload call: on any non-IOException failure, delete the batch, report via onError with FlushFailed, return true. Backoff test updated to cover the restored 40s step. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
a527ba9 to
d68e4e7
Compare
nattb8
approved these changes
Apr 21, 2026
ImmutableJeffrey
added a commit
that referenced
this pull request
Apr 21, 2026
Addresses review feedback on PR #691. The previous catch (Exception) in BuildPayload swallowed every failure as if it were the expected file-disappeared race. Permissions errors (UnauthorizedAccessException) and similar non-IO faults were silently dropped, masking real problems. Narrowing to IOException keeps the file-disappeared / locked / path- gone cases skipped — which is what the retry loop needs — while letting genuinely broken states propagate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ImmutableJeffrey
added a commit
that referenced
this pull request
Apr 21, 2026
Addresses review feedback on PR #691. Makes the nullable return value explicit in the XML doc — the reviewer asked for Passport-style clarity on what's nullable. Enabling nullable reference types package-wide is a larger follow-up; this commit covers the specific case flagged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Adds
HttpTransportand theGziphelper that compresses event batches before upload.What it does
DiskStore, compresses them, and uploads them to the audience backend (/v1/audience/messages) on a background thread.Retry behaviour
onErrorcallback asFlushFailed, rather than silently crashing the background flush loop.Caller-facing surface
NextAttemptAtandIsInBackoffWindowexpose retry timing so a scheduler outside the transport can decide when to call again.onErrorcallback with one of three codes:ValidationRejected,FlushFailed,NetworkError. Exceptions thrown inside the callback are caught.Linear: SDK-141