-
Notifications
You must be signed in to change notification settings - Fork 45
chore: Support x-ld-envid in updates #370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| state=DataSourceState.OFF, | ||
| revert_to_fdv1=True | ||
| revert_to_fdv1=True, | ||
| environment_id=envid, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Start Action Leaves Environment State Stale
When a Start action arrives with headers=None, the envid variable isn't updated, causing it to retain a stale value from a previous Fault action. This means subsequent event updates could incorrectly use an environment_id that was only meant for the fault's error update. The condition if isinstance(action, Start) and action.headers is not None: should update envid regardless of whether headers exist, setting it to None when headers are absent to properly reflect the new connection state.
| continue | ||
|
|
||
| (update, should_continue) = self._handle_error(action.error) | ||
| envid = action.headers.get(_LD_ENVID_HEADER) if action.headers is not None else None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Fault object: Headers in the wrong place.
The code attempts to access action.headers on a Fault object, but Fault objects don't have a headers attribute. The headers are stored on the error object (action.error.headers), as evidenced by the fallback logic in _handle_error at line 354-355 which correctly accesses error.headers. This will cause an AttributeError when a Fault with an error containing headers is encountered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good point. Fixed.
Note
Propagates X-LD-EnvID/FD-Fallback headers through polling/streaming to set Update.environment_id and trigger FDv1 fallback; refactors error/header handling and expands tests, with minor build tweaks.
_LD_ENVID_HEADER/_LD_FD_FALLBACK_HEADERto readX-LD-EnvIDand FD fallback, attachingenvironment_idto allUpdates and honoring fallback in both success and error paths.environment_idacross stream events; include it in INTERRUPTED/OFF states; handle Start/Fault headers and recoverable/unrecoverable errors consistently.headerswith failures; parsing failures also return headers._LD_ENVID_HEADER/_LD_FD_FALLBACK_HEADERconstants.UnsuccessfulResponseExceptionno longer stores headers;_Failgains optionalheaders.environment_idpropagation and FD fallback across polling and streaming scenarios.TestDataV2builders directly..mypy_cacheexists before running mypy.pytestto^8.0.0.Written by Cursor Bugbot for commit 94c2334. This will update automatically on new commits. Configure here.