Skip to content

fix: Keep the override marking when a migration flag serves an unrecognized stage - #492

Merged
kinyoklion merged 1 commit into
v7from
rlamb/overrides-go-migration-marker
Oct 29, 2026
Merged

kinyoklion merged 1 commit into
v7from
rlamb/overrides-go-migration-marker

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Summary

When a migration flag evaluates to a string that is not a migration stage, migrationVariation replaces the detail with a WRONG_TYPE error for the default stage. The replacement dropped the evaluation's overrideAffected marking, so an overridden migration flag with an unrecognized stage gave the MigrationOpTracker an unmarked reason. OVERRIDE 1.5.4 marks an evaluation that fails when a definition it read came from the override store. The same review finding was made on the .NET port (dotnet-core #364).

The new reason now keeps the marking, the same way variationAndFlag keeps it when a requested-type mismatch replaces the reason. A migration flag that serves a value that is not a string already kept it: that path goes through variationAndFlag's type check, and the default stage then parses. Evaluation events are unchanged, because they are recorded before the stage is parsed and carry the marking from the evaluation result.

Tests

ldclient_overrides_test.go: an overridden migration flag that serves "not-a-stage" returns the default stage and an error, and the operation event that its tracker builds carries a WRONG_TYPE reason marked overrideAffected. It fails before the change and when the fix is reverted. go test -race ./... passes; make lint reports no issues.

Opened from termag


Note

Overview
When MigrationVariation gets a string that is not a valid migration stage, it replaces the evaluation detail with a WRONG_TYPE error and the default stage. That replacement used to drop overrideAffected, so migration operation events from MigrationOpTracker could look like a normal (non-override) failure even when the value came from the override layer.

The fix copies the prior reason’s overrideAffected flag onto the new error reason—the same pattern variationAndFlag already uses for type mismatches. Standard flag evaluation events are unchanged; they are emitted before stage parsing.

A test covers an overridden migration flag serving "not-a-stage" and asserts the built operation event’s reason is WRONG_TYPE with overrideAffected: true.

Reviewed by Cursor Bugbot for commit 36def3b. Bugbot is set up for automated code reviews on this repo. Configure here.

@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@kinyoklion
kinyoklion merged commit 9f9911f into v7 Oct 29, 2026
32 checks passed
@kinyoklion
kinyoklion deleted the rlamb/overrides-go-migration-marker branch October 29, 2026 15:43
beekld pushed a commit that referenced this pull request Nov 6, 2026
🤖 I have created a release *beep* *boop*
---


##
[7.22.0](v7.21.2...v7.22.0)
(2026-11-06)


### Features

* Broadcast Off to FDv2 status listeners when the data system stops
([#505](#505))
([5c5b8ce](5c5b8ce))
* End an FDv1 fallback after a default TTL of one hour
([#499](#499))
([1f03a5f](1f03a5f))
* Park FDv2 synchronizers instead of retiring them on unexpected
failures
([#489](#489))
([644253a](644253a))


### Bug Fixes

* Keep data from a store that is not initialized out of override
evaluations and AllFlagsState, and report a store failure
([#491](#491))
([1005a1a](1005a1a))
* Keep the override marking when a migration flag serves an unrecognized
stage ([#492](#492))
([9f9911f](9f9911f))
* Leave StateSince alone when a synchronizer starts
([#502](#502))
([823b9aa](823b9aa))
* Make TestData ValueForAll return its value when targeting is off
([#507](#507))
([7300bc1](7300bc1))
* Park a synchronizer that reports Off or closes its results
([#500](#500))
([f6e9910](f6e9910))
* Poll the override files when they cannot be watched
([#495](#495))
([170ad3b](170ad3b))
* Prevent a full status listener from hanging Close
([#498](#498))
([9efd570](9efd570))
* Say that the data system retries after an unrecoverable HTTP error
([#501](#501))
([d4066cc](d4066cc))
* Treat a repeated override file path in a different case as a repeat on
Windows
([#494](#494))
([eeb68d3](eeb68d3))
* Warn when offline mode skips the override source
([#493](#493))
([69bbe61](69bbe61))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Overview**
> **Cuts the Go server SDK release `7.22.0` and records its notes.**
> 
> The root **`.release-please-manifest.json`** and
**`internal/version.go`** now declare **`7.22.0`** (replacing `7.21.2`).
**`CHANGELOG.md`** gains a new **`[7.22.0]`** section dated
**2026-11-06** documenting shipped work: FDv2 data-system behavior
(status **Off** broadcasts, synchronizer **parking** instead of retire,
**FDv1 fallback TTL**, **StateSince** handling, parking on
**Off**/closed results), plus multiple **bug fixes** around
uninitialized store data in overrides/**AllFlagsState**, migration
override marking, **TestData** when targeting is off, **override file**
polling/deduping on Windows, offline-mode warnings, status listener
**Close** hangs, and messaging for unrecoverable HTTP retries.
> 
> No application code changes appear in this PR; it’s version
bookkeeping and release notes only.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
c763cc0. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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