Skip to content

fix(audience): Init does not mutate caller's AudienceConfig (SDK-235)#702

Closed
ImmutableJeffrey wants to merge 3 commits into
mainfrom
feat/audience-init-hygiene
Closed

fix(audience): Init does not mutate caller's AudienceConfig (SDK-235)#702
ImmutableJeffrey wants to merge 3 commits into
mainfrom
feat/audience-init-hygiene

Conversation

@ImmutableJeffrey
Copy link
Copy Markdown
Collaborator

@ImmutableJeffrey ImmutableJeffrey commented Apr 23, 2026

Summary

  • Adds internal AudienceConfig.Clone() backed by MemberwiseClone. Reference-typed properties (OnError, HttpHandler) remain shared — cloning delegates or handlers would break them.
  • Clones the caller's config inside Init before writing the resolved PersistentDataPath, so callers who reuse a config (tests, retry flows, domain-reload scenarios) no longer observe a mutated field.

Linear: SDK-235

Init auto-fills PersistentDataPath from DefaultPersistentDataPathProvider.
The old code wrote that back onto the caller's config object — a
surprising side-effect for callers who reuse the config.

Clone via MemberwiseClone before the fill-in. Reference-typed properties
(OnError, HttpHandler) are intentionally shared — cloning delegates
and handlers would break them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ImmutableJeffrey ImmutableJeffrey requested review from a team as code owners April 23, 2026 04:55
@ImmutableJeffrey ImmutableJeffrey force-pushed the feat/audience-init-hygiene branch from 80be498 to f8fda34 Compare April 23, 2026 05:13
Rewrites the Clone() rationale and the Init call-site comment in
plain language — no jargon about "shallow copy" or "caller's object".

No code changes. All 180 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ImmutableJeffrey ImmutableJeffrey force-pushed the feat/audience-init-hygiene branch 2 times, most recently from 84eb376 to ba7b793 Compare April 23, 2026 05:22
The previous order (validate then Clone) reset the compiler's
null-flow analysis when config was reassigned to the clone, so the
later `new HttpTransport(..., config.PublishableKey, ...)` call
emitted CS8604 "Possible null reference argument for parameter
'publishableKey'".

Moving the Clone above the PublishableKey check keeps validation on
the same config object used downstream. MemberwiseClone is O(1), so
the extra allocation on the error path is trivial.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ImmutableJeffrey ImmutableJeffrey marked this pull request as draft April 23, 2026 05:53
@nattb8 nattb8 marked this pull request as ready for review April 23, 2026 05:56
@ImmutableJeffrey ImmutableJeffrey marked this pull request as draft April 23, 2026 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants