-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add Request::into_http
#11843
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
Add Request::into_http
#11843
Conversation
dicej
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this; LGTM.
Please run cargo fmt when you have a chance, which should address the CI failure.
|
Would it also be possible to update the |
No problem! Seems a fair exchange for the past/future questions I'll have about
I looked into this, but I think I might not quite understand the ask as the Edit: Ah, guessing you meant this code which eventually calls |
|
Ah yes indeed! That eventually bottoms out here and I think yeah you're right that what you've added here is more suitable for replacing/augmenting the implementation of |
|
@alexcrichton So I looked into it a bit, and given my limited understanding of Might need a bit of hand holding here 😅 |
|
Ah ok no worries, I can try to take a look after this merges |
|
Looks like it leaked some memory during testing as well? 😞 =================================================================
==18584==ERROR: LeakSanitizer: detected memory leaks
Indirect leak of 72 byte(s) in 1 object(s) allocated from:
#0 0x563417655224 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x14cb224) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#1 0x563417756761 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15cc761) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#2 0x5634176fc8f9 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15728f9) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#3 0x5634177a923d (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x161f23d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#4 0x5634176ae4da (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15244da) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#5 0x56341771965c (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x158f65c) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#6 0x56341775d61d (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15d361d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#7 0x5634176ad96d (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x152396d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#8 0x5634177a894d (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x161e94d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#9 0x5634177c1c98 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1637c98) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#10 0x5634177ffef4 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1675ef4) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#11 0x5634177d68e3 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x164c8e3) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#12 0x5634177da219 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1650219) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#13 0x563419c47c6e (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x3abdc6e) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#14 0x563417652b96 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x14c8b96) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
Indirect leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x563417655224 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x14cb224) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#1 0x5634176fdb34 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1573b34) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#2 0x5634177a95f7 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x161f5f7) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#3 0x5634176ae4da (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15244da) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#4 0x56341771965c (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x158f65c) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#5 0x56341775d61d (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15d361d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#6 0x5634176ad96d (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x152396d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#7 0x5634177a894d (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x161e94d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#8 0x5634177c1c98 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1637c98) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#9 0x5634177ffef4 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1675ef4) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#10 0x5634177d68e3 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x164c8e3) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#11 0x5634177da219 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1650219) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#12 0x563419c47c6e (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x3abdc6e) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
#13 0x563417652b96 (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x14c8b96) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
SUMMARY: AddressSanitizer: 104 byte(s) leaked in 2 allocation(s).
error: test failed, to rerun pass `-p wasmtime-wasi-http --lib`
Caused by:
process didn't exit successfully: `/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521` (exit status: 1)
note: test exited abnormally; to see the full output pass --no-capture to the harness.
Error: Process completed with exit code 1. |
|
As for the leak, it looks like that's caused by the test added here so I don't think it's spurious. The error message isn't great, however, but if you try running locally it might provide a better error message when |
|
Thanks for the detailed review and guidance @rvolosatovs! I'll incorporate the changes when I get a chance. |
|
Question for y'all: the new method here looks to more-or-less be a copy/paste of this functionality. Would it be possible to unify the two, and if not, how come? The handling of |
I'm working on a PR, which will align the two |
|
See #11970 |
|
Makes sense, thanks! |
Discussion: #wasi > wasi-http Request and pub(crate) Body @ 💬
Largely copies the implementation from
Response::into_httpas suggested, contains one basic test asserting it works in the happy path.