Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve full request context while redirecting #3104

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

idelpivnitskiy
Copy link
Member

Motivation:

Redirect handling creates new requests, but copies only HTTP_EXECUTION_STRATEGY_KEY. As the result, next filters after redirect don't see a state associated with the request flow.

Modifications:

  • Copy full ContextMap from each previous request to the next one;

Result:

Any context information is preserved for all redirected requests, like UUID, counters, execution strategy, etc.

Motivation:

Redirect handling creates new requests, but copies only
`HTTP_EXECUTION_STRATEGY_KEY`. As the result, next filters after
redirect don't see a state associated with the request flow.

Modifications:

- Copy full `ContextMap` from each previous request to the next one;

Result:

Any context information is preserved for all redirected requests, like
UUID, counters, execution strategy, etc.
@idelpivnitskiy idelpivnitskiy self-assigned this Nov 14, 2024
Copy link
Contributor

@daschl daschl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good to me, just to double check: I assume there is no context that we do not want to carry over after a redirect?

@idelpivnitskiy
Copy link
Member Author

@daschl I don't have a use-case in mind when ppl won't want some context to propagate forward. Context is local to the app and does not leak over wire. Should be fine to copy all. If there is a use-case, users can always use request transformer to clean some parts of the context they don't want to propagate or if they want to reset something there.

@idelpivnitskiy idelpivnitskiy merged commit 8d30706 into apple:main Nov 14, 2024
11 checks passed
@idelpivnitskiy idelpivnitskiy deleted the redirect-ctx branch November 14, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants