Skip to content

feat(audience): HttpTransport — background POST with gzip + retry (SDK-141)#691

Merged
ImmutableJeffrey merged 7 commits into
mainfrom
feat/sdk-141-http-transport
Apr 21, 2026
Merged

feat(audience): HttpTransport — background POST with gzip + retry (SDK-141)#691
ImmutableJeffrey merged 7 commits into
mainfrom
feat/sdk-141-http-transport

Conversation

@ImmutableJeffrey
Copy link
Copy Markdown
Collaborator

@ImmutableJeffrey ImmutableJeffrey commented Apr 17, 2026

Summary

Adds HttpTransport and the Gzip helper that compresses event batches before upload.

What it does

  • Reads batches of queued events from DiskStore, compresses them, and uploads them to the audience backend (/v1/audience/messages) on a background thread.
  • Routes test API keys to sandbox and production keys to production.

Retry behaviour

  • Backend accepts (2xx) → delete the batch.
  • Backend rejects (4xx) → delete the batch (re-sending bad data won't help).
  • Backend has problems (5xx) or network is down → keep the batch on disk and back off before retrying. Wait grows 5s → 10s → 20s → 40s, capped at 60s. Polling more often doesn't compound the wait — escalation only happens once the previous wait has passed.
  • Local file read fails (e.g. permissions stripped) → drop the batch and report through the onError callback as FlushFailed, rather than silently crashing the background flush loop.

Caller-facing surface

  • NextAttemptAt and IsInBackoffWindow expose retry timing so a scheduler outside the transport can decide when to call again.
  • Failures fire an optional onError callback with one of three codes: ValidationRejected, FlushFailed, NetworkError. Exceptions thrown inside the callback are caught.

Linear: SDK-141

@ImmutableJeffrey ImmutableJeffrey requested review from a team as code owners April 17, 2026 02:53
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread src/Packages/Audience/Runtime/Transport/HttpTransport.cs
@ImmutableJeffrey ImmutableJeffrey force-pushed the feat/sdk-141-http-transport branch from f66dff6 to b9faf5a Compare April 17, 2026 03:00
ImmutableJeffrey and others added 3 commits April 17, 2026 13:19
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>
@ImmutableJeffrey ImmutableJeffrey force-pushed the feat/sdk-141-http-transport branch from b9faf5a to 52381a4 Compare April 17, 2026 03:22
Comment thread src/Packages/Audience/Runtime/Transport/HttpTransport.cs
Comment thread src/Packages/Audience/Runtime/Transport/HttpTransport.cs Outdated
Comment thread src/Packages/Audience/Runtime/Transport/HttpTransport.cs Outdated
ImmutableJeffrey and others added 4 commits April 18, 2026 11:07
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>
@ImmutableJeffrey ImmutableJeffrey force-pushed the feat/sdk-141-http-transport branch from a527ba9 to d68e4e7 Compare April 21, 2026 02:58
@ImmutableJeffrey ImmutableJeffrey merged commit 61baf7a into main Apr 21, 2026
18 checks passed
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>
@ImmutableJeffrey ImmutableJeffrey deleted the feat/sdk-141-http-transport branch April 21, 2026 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants