fix: track Ponder fallback routing as a flag, not a URL comparison - #63
Closed
Danswar wants to merge 2 commits into
Closed
fix: track Ponder fallback routing as a flag, not a URL comparison#63Danswar wants to merge 2 commits into
Danswar wants to merge 2 commits into
Conversation
Ported from d-EURO#123. sentToFallback compared the request's targetUrl against CONFIG.indexerFallback by string value. This repo's own deploy config sets CONFIG_INDEXER_FALLBACK_URL to the same host as CONFIG_INDEXER_URL in both environments — the way to keep the var populated without cross-environment failover. With primary and fallback string-identical, the 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. Stamp the routing decision itself (isFallbackActive() at send time) instead of re-deriving it from a URL string.
2 tasks
"prd/dev" echoes internal server naming shorthand that doesn't belong in a public repo; reword to a generic reference.
Author
|
3 review passes to 0 findings. This ports an already-verified fix (d-EURO#124) unchanged, so logic passed clean on the first pass. Two follow-up passes on wording caught and fixed public-repo hygiene issues in the PR description, commit message, and a code comment — internal terminology replaced with generic phrasing. Final state: no outstanding issues, CI green. |
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
Ports d-EURO#124 (same
api.apollo.config.ts, forked codebase).errorLinkdecided whether a failed Ponder request already went to the fallback by comparingoperation.getContext().targetUrlagainstCONFIG.indexerFallbackas strings.CONFIG_INDEXER_FALLBACK_URLto the same host asCONFIG_INDEXER_URLin both environments (the way to keep the var populated without leaking traffic across environments). With primary and fallback string-identical, the comparison is true on every request regardless of which URL it actually went to, so thewarn+ retry branch could never execute — every network error would still fall through tologger.error, making the deploy-config change alone insufficient to fix the noisy error-level logging observed for this service.routingLinknow stampsusedFallback: isFallbackActive()at send time, anderrorLinkreads that flag instead of re-deriving it from a URL comparison.Test plan
routingLinkre-stamps context on retry from the now-fallback-active state, so a second failure against the fallback still correctly falls through toerror. Confirmed this repo's ownapi.config.tssources the same env vars as d-EURO/api, so the fix is not inert here.errortowarnon the next transient Ponder blip.(Supersedes #62, closed — that PR's description/commit message referenced internal infra details that don't belong in a public repo.)