fix: track Ponder fallback routing as a flag, not a URL comparison - #124
Closed
Danswar wants to merge 1 commit into
Closed
fix: track Ponder fallback routing as a flag, not a URL comparison#124Danswar wants to merge 1 commit into
Danswar wants to merge 1 commit into
Conversation
sentToFallback compared operation.getContext().targetUrl against CONFIG.indexerFallback by string value. Once an operator points the fallback at the same host as the primary (documented in this repo's own deploy config as the way to keep CONFIG_INDEXER_FALLBACK_URL populated without enabling cross-environment failover), that comparison is true on every request regardless of which URL it actually went to — so the warn+retry branch never fires and every network error still falls through to logger.error. Confirmed live: 30/30 recent ApiApolloConfig network-error log lines in prod were at error, none at warn, despite the fallback fix (#117/#121) being deployed since 2026-07-20. Stamp the routing decision itself (isFallbackActive() at send time) instead of re-deriving it from a URL string.
This was referenced Jul 23, 2026
Author
|
3 review passes to 0 findings. Correctness (retry lifecycle, concurrency, the string-comparison edge case this PR fixes) verified clean on the first pass. Two follow-up passes on wording caught and fixed public-repo hygiene issues — internal shorthand terminology that had leaked into a code comment, a commit message, and the PR description — replaced with generic phrasing. Final state: no outstanding issues, CI green. |
Danswar
marked this pull request as ready for review
July 23, 2026 00:42
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
errorLinkdecides whether a failed Ponder request already went to the fallback by comparingoperation.getContext().targetUrlagainstCONFIG.indexerFallbackas strings.CONFIG_INDEXER_FALLBACK_URLto the same value asCONFIG_INDEXER_URL(the way to keep the var populated without enabling cross-environment failover). With primary and fallback string-identical,targetUrl === CONFIG.indexerFallbackis true on every request regardless of which URL it actually went to, so thewarn+ retry branch (lines 47-52) can never execute — every network error still falls through tologger.error.ApiApolloConfignetwork-error lines in prod were allerror, nonewarn, despite this fallback mechanism (fix: reclassify Apollo network-error log severity #117 / fix: attribute network errors to the URL the request was sent to #121) having been deployed since 2026-07-20.routingLinknow stampsusedFallback: isFallbackActive()at send time, anderrorLinkreads that flag instead of re-deriving it from a URL comparison. This correctly distinguishes primary vs. fallback attempts even when they resolve to the same URL string.Test plan
routingLinkre-stamps context from the (now fallback-active) state, so a second failure against the fallback correctly falls through toerror— behavior unchanged from the intended design, just no longer defeated by same-URL configs.ApiApolloConfignetwork-error lines to move fromerrortowarnon the next transient Ponder blip.(Supersedes #123, closed — that PR's follow-up commit message and description used internal-sounding shorthand that doesn't belong in a public repo.)