Fix async redirect handling regression from FOLLOW_REDIRECTS default#1322
Draft
cursor[bot] wants to merge 3 commits into
Draft
Fix async redirect handling regression from FOLLOW_REDIRECTS default#1322cursor[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
2 participants
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.
Bug and impact
Commit #1278 introduced
FOLLOW_REDIRECTS, defaulting redirect following totruefor all request modes. That regressed async connectors such as Slack Analytics, which rely on the proxy intercepting 3xx responses and manually fetching theLocationURL. With automatic redirect following enabled, the HTTP client follows redirects before the async redirect-handling block runs, causing async exports to fail or return incorrect data.ChatGPT Enterprise was protected because its connector spec explicitly sets
FOLLOW_REDIRECTS=FALSE, but Slack Analytics (enable_async_processing: true, noFOLLOW_REDIRECTSenv var) and other custom async connectors were broken.Root cause
ApiDataRequestHandlerchanged from.setFollowRedirects(!processingContext.getAsync())to:This made sync-mode behavior the default for async requests unless the env var was explicitly set.
Fix and validation
orElse(!processingContext.getAsync())— async requests do not follow redirects unlessFOLLOW_REDIRECTSis explicitly configured.ProxyConfigPropertyjavadoc to document the mode-specific defaults.asyncModeDisablesRedirectFollowingByDefaultunit test to lock in the behavior.Validation:
mvn test -pl core -Dtest=ApiDataRequestHandlerTest#asyncModeDisablesRedirectFollowingByDefault,ApiDataRequestHandlerTest#handleShouldFollowRedirectManuallyInAsyncMode,ApiDataRequestHandlerTest#handleShouldLetHttpClientFollowRedirectInSyncMode— all pass.