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

Cancel MITM request context when its handling finishes #634

Merged
merged 3 commits into from
Jan 21, 2025

Conversation

ErikPelli
Copy link
Collaborator

Fixes #633.
Add Go context cancel to a custom parsed request, since we handle it.

@ErikPelli
Copy link
Collaborator Author

@alessiodallapiazza I tested your example script with this release and it works perfectly.
If it's all good for you, I think I can merge this pull request as a stable release, since we're also passing the test pipeline without issues.
What do you think?

@alessiodallapiazza
Copy link
Contributor

I’ve run some tests as well, and everything looks good on my end. 👍

@ErikPelli ErikPelli merged commit aa6e8d3 into master Jan 21, 2025
1 check passed
@ErikPelli ErikPelli deleted the fix-request-context branch January 21, 2025 10:29
@thisislawatts
Copy link

@ErikPelli why are automated test cases not being introduced to validate the behaviour modifications that are being made here?

@ErikPelli
Copy link
Collaborator Author

I don't think we need to introduce new tests for every single change we made, do you think they are necessary for this one?

@thisislawatts
Copy link

thisislawatts commented Jan 28, 2025

This specific change was made to address a specific issue #633. So I guess if it was a significant enough bug to warrant a change it also seems worth capturing the desired behaviour in an automated test. Otherwise, I am curious how you would avoid introducing regressions or reoccurrences of the same bug?

@ErikPelli
Copy link
Collaborator Author

Added the test in another pull request

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.

Context Done Not Propagated in Goproxy Extension Limiter During HTTPS/CONNECT
3 participants