A caller mutates Config.Headers after passing the Config by value into your library's New - why does that still change your client's behaviour, and what do you change?

## Why by-value did not protect you `func New(cfg Config) *Client` copies the `Config` struct into the constructor's frame. That copy is one level deep: `Endpoint string` is now yours, but `Headers map[string]string` is a copy of a pointer to the caller's map. Storing `cfg` in the client stores that same reference. The caller kept theirs. You now have two owners of one mutable object and no agreement about who may write to it. The symptom is usually not a clean bug report. It arrives as "the client sometimes sends the wrong Authorization header", or as a race detector report in the caller's tests pointing at your package, or as a config change that mysteriously applies to clients created before it. All three come from the same place. The severity is worth stating plainly: it is not only surprising, it is a **data race**. A map read in your request path concurrent with a caller's write is undefined behaviour in Go, and a concurrent map write can crash the process outright with a fatal error that no `recover` will catch. ## The snapshot boundary The library author's job here is to decide, once, where the value stops being the caller's and starts being yours - and then to make the code enforce it: ```go func New(cfg Config) *Client { cfg.Headers = maps.Clone(cfg.Headers) // snapshot: the caller keeps theirs cfg.Retries = slices.Clone(cfg.Retries) if cfg.Timeout == 0 { cfg.Timeout = defaultTimeout } return &Client{cfg: cfg} } ``` Three properties make this the right default for a library other teams import: 1. **It is cheap and it happens once.** Construction is not a hot path; two allocations per client are irrelevant next to one request. 2. **It is local.** Nothing about the caller's code has to change, and no documentation has to be obeyed for correctness. 3. **It makes the defaults coherent.** You are already normalising zero fields into defaults in `New`; taking the snapshot in the same place means the config the client holds is exactly the config it will use forever. The mirror image applies on the way out. A method that returns the internal map hands the caller a live handle on your state, with the same race. Return a clone, return a copy of the individual value, or do not expose it. ## What not to clone Deep copying everything is the beginner's overcorrection and it breaks real behaviour. Per field: - **data the caller may mutate** - maps, slices, pointers to plain structs: clone - **collaborators** - `*log.Logger`, `*sql.DB`, an `http.RoundTripper`, a channel, a `context.Context`: share, always. Cloning a logger gives you a second logger; cloning a pool handle is usually not even possible - **immutable scalars** - strings, durations, times: plain assignment is already a snapshot - **anything containing a lock**: it must not be copied at all, which also means a `Config` should not embed a mutex The useful phrasing for a review is "clone the data, share the dependencies". ## The alternatives, and when each wins - **Take `*Config`.** Honest about the aliasing rather than hiding it, but it pushes the whole problem onto the caller and every caller must now think about lifetime. Worse for an exported API. - **Functional options** (`New(url, WithHeader(k, v))`). The caller never holds the mutable structure, so there is nothing to alias. Costs more API surface; strong when the option set is large or grows over time. - **Document the rule and clone nothing.** Cheapest to write, and it fails the moment one caller does not read the comment. Acceptable only for an internal package with a handful of known call sites. - **Freeze into an immutable internal type.** Convert `Config` into an unexported struct with no reference fields where possible, which makes the snapshot structural rather than a line of code somebody can delete. ## Making the decision stick Whichever you pick, two things travel with it. First, the doc comment on `New` says whether the library retains a reference to anything the caller passed - this is the single sentence that resolves the ambiguity for every future caller. Second, a test proves it: build a client, mutate every field of the caller's config afterwards, and assert the client's behaviour is unchanged. That test is what stops the snapshot being quietly dropped when somebody adds a field two years later. The underlying principle is the one that makes this a design question rather than a trick: in Go, a value type is not the same thing as an immutable one, so **passing by value is not an ownership boundary**. You have to draw the boundary yourself.