Fix async redirect handling regression from FOLLOW_REDIRECTS default#1317
Closed
cursor[bot] wants to merge 3 commits into
Closed
Fix async redirect handling regression from FOLLOW_REDIRECTS default#1317cursor[bot] wants to merge 3 commits into
cursor[bot] wants to merge 3 commits into
Conversation
* drop lookup buckets from CallerAccess policy * style fixes
Commit #1278 changed redirect following to default true unless FOLLOW_REDIRECTS is explicitly set. That broke async connectors (e.g. Slack Analytics) which rely on intercepting 3xx responses when FOLLOW_REDIRECTS is not configured. Restore the prior default: do not follow redirects in async processing unless FOLLOW_REDIRECTS is explicitly set. Add a unit test that locks in the behavior. Co-authored-by: Erik Schultink <eschultink@users.noreply.github.com>
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.
Fixes
FOLLOW_REDIRECTSis not configuredChange implications
FOLLOW_REDIRECTSis unset, redirects are now again disabled in async mode (restoring pre-Adding property config for redirect #1278 behavior) so the proxy can manually fetch from theLocationheader. Only ChatGPT Enterprise had an explicitFOLLOW_REDIRECTS=FALSEoverride in Terraform; other async connectors like Slack Analytics were affected.Bug and impact
Commit #1278 (
Added property) changedApiDataRequestHandlerto defaultFOLLOW_REDIRECTStotruefor all requests, replacing the prior behavior ofsetFollowRedirects(!processingContext.getAsync()). Async connectors such as Slack Analytics depend on the proxy intercepting 3xx responses and manually fetching from theLocationURL (with HTTPS validation). With auto-follow enabled, that interception block is bypassed, breaking async data collection for connectors that do not have an explicitFOLLOW_REDIRECTS=FALSEenvironment variable.Root cause
#1278 introduced a configurable
FOLLOW_REDIRECTSproperty but changed the default from context-aware (falsein async mode) to unconditionallytrue, removing the async-specific redirect handling added in #1236.Fix and validation
orElse(!processingContext.getAsync())as the default whenFOLLOW_REDIRECTSis unsetProxyConfigPropertyjavadoc to document the async/sync default differenceasyncModeDisablesRedirectFollowingByDefaultunit testmvn test -pl core -am -Dtest=ApiDataRequestHandlerTest#asyncModeDisablesRedirectFollowingByDefault,ApiDataRequestHandlerTest#handleShouldFollowRedirectManuallyInAsyncMode,ApiDataRequestHandlerTest#handleShouldLetHttpClientFollowRedirectInSyncMode— all pass