-
Notifications
You must be signed in to change notification settings - Fork 289
factor-outbound-http: Refactor wasi impls #3268
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
Merged
Merged
Conversation
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
3e3826b to
9798cf0
Compare
dicej
approved these changes
Sep 11, 2025
fibonacci1729
approved these changes
Sep 12, 2025
Collaborator
Author
|
Bisecting, the test failure surprisingly happens in the first commit. 🤔 |
Collaborator
Author
|
OK I'm pretty sure this is just a subtle change to the way errors got propagated with the nested |
9798cf0 to
b1d07da
Compare
Collaborator
Author
|
Squashing that bug inspired/compelled me to do another round of refactoring to get rid of the nested |
80e0b16 to
6f6c50c
Compare
Signed-off-by: Lann Martin <[email protected]>
Signed-off-by: Lann Martin <[email protected]>
Signed-off-by: Lann Martin <[email protected]>
Signed-off-by: Lann Martin <[email protected]>
Signed-off-by: Lann Martin <[email protected]>
Trying to debug a flaky CI test failure. Signed-off-by: Lann Martin <[email protected]>
6f6c50c to
c9d2611
Compare
Collaborator
Author
|
The last CI run succeeded; I just needed to sign my commits. |
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.
Putting my commits where my fingers type.
This is split into
35 commits to ease review:send_request_impl, inliningsend_request_handlerand making it a method on a new struct that holds most of its inputs. This doesn't do anything functionally but I think it helps distinguish the data that was copied from the instance state and may aid future refactoring.TLS_CLIENT_CONFIGtask-local intoCONNECT_OPTIONS, makesconnect_tcpandconnect_tlsmethods onConnectOptions, and makes a bunch of miscellaneous simplifications (which I now realize could/should have done in a fourth commit; sorry!).sendmethod into a few parts to help avoid confusion around error handling.