fix(analytics): surface polling-loop errors so schema drift can't loop silently (SDK-78)#156
Closed
tylerjroach wants to merge 1 commit into
Closed
fix(analytics): surface polling-loop errors so schema drift can't loop silently (SDK-78)#156tylerjroach wants to merge 1 commit into
tylerjroach wants to merge 1 commit into
Conversation
…p silently start_polling_for_definitions! caught StandardError and dispatched only to @error_handler (whose default ErrorHandler#handle is a no-op), so NoMethodError / JSON::ParserError / etc. from schema drift would loop forever undetected unless the user had configured a custom handler. Add log_polling_error which always warns to STDERR before dispatching to @error_handler — visibility no longer depends on configuration. Matches the convention in mixpanel-python, mixpanel-java, mixpanel-go, and mixpanel-node, all of which log unconditionally and continue polling. Linear: SDK-78 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #156 +/- ##
==========================================
+ Coverage 96.64% 96.66% +0.01%
==========================================
Files 14 14
Lines 656 659 +3
==========================================
+ Hits 634 637 +3
Misses 22 22
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Folded into #155 — both fixes touch the same begin/rescue block in start_polling_for_definitions!, simpler as one PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
start_polling_for_definitions!caughtStandardErrorand dispatched only to@error_handler, whose defaultMixpanel::ErrorHandler#handleis a no-op. Schema drift (NoMethodError,JSON::ParserError,TypeError, …) would loop forever undetected unless the user had configured a custom handler — exactly the audit's concern.Add a
log_polling_errorhelper that alwayswarns to$stderrbefore dispatching to@error_handler. Visibility no longer depends on configuration; operators see the failure in their logs the moment the next poll hits a bad payload.Why a
warnand not a re-raiseMatches the de facto convention across every other Mixpanel SDK polling loop:
logger.exception("Failed to fetch feature flag definitions"), continuelogger.log(Level.WARNING, "Failed to fetch flag definitions", e), continuelog.Printf("Error polling for flag definitions: %v", err), continuethis.logger?.error(...), continueCrashing the polling thread on the first bad payload would freeze local evaluation at whatever variants were last cached — strictly worse than continuing. The audit's complaint was about detection, not retry policy.
Context
Linear: SDK-78. Source: Feature Flags & OpenFeature SDK Audit Findings. Ruby was the only affected SDK — the rest already log unconditionally.
Independent of SDK-81 (the polling-thread-safety fix); changes are in different lines of the same
begin/rescueblock and will merge cleanly in either order.Test plan
"warns to stderr when fetch raises and no error_handler is configured"exercises the audit-failure mode directly (provider built withnilerror_handler + 500 response, asserts thewarnlands on stderr)"surfaces unexpected errors (schema drift) instead of swallowing them silently"injects aNoMethodErrorviaallow(...).to receive(:fetch_flag_definitions), asserts both the error_handler dispatch AND the stderr warning fire🤖 Generated with Claude Code