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.

Sign up for GitHub

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

Jump to bottom

Fix HttpTransformer-related issues #2016

Merged
merged 4 commits into from
Jan 31, 2023
Merged

Fix HttpTransformer-related issues #2016

merged 4 commits into from
Jan 31, 2023

Conversation

MihaZupan
Copy link
Member

@MihaZupan MihaZupan commented Jan 30, 2023 •
edited
Loading

This PR does a few things:

  • Marks HttpTransformer overloads without the CancellationToken as obsolete.
  • Makes sure all scenarios work as expected with the built-in StructuredTransformer (you can call overloads with or without the CT and it'll work).
  • Makes sure HttpForwarder always calls the overloads with the cancellation token.
  • Moves request transform calls into the try/catch block with HttpClient.SendAsync so that any failures are reported in a similar manner. Also added ForwarderError.RequestCreation in case the exception is not related to cancellation.
  • Switched the error reported if the manual CT is signaled to RequestCanceled (was RequestTimedOut).
  • Added a bunch of tests for the above.

@MihaZupan MihaZupan added this to the YARP 2.0.0 milestone Jan 30, 2023
@MihaZupan MihaZupan self-assigned this Jan 30, 2023
@MihaZupan MihaZupan merged commit 79a7b80 into microsoft:main Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tratcher Tratcher Tratcher approved these changes

Assignees

@MihaZupan MihaZupan

Labels
Breaking-Change
Projects
None yet
Milestone
YARP 2.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@MihaZupan @Tratcher