http: add asyncRequest #2877
http: add asyncRequest #2877
Conversation
mstoykov
force-pushed
the
httpAsyncRequest
branch
from
0f22696
to
9d72974
Compare
Codecov Report
@@ Coverage Diff @@
## master #2877 +/- ##
==========================================
+ Coverage 76.71% 76.86% +0.15%
==========================================
Files 225 225
Lines 16807 16869 +62
==========================================
+ Hits 12894 12967 +73
+ Misses 3085 3067 -18
- Partials 828 835 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
mstoykov
force-pushed
the
httpAsyncRequest
branch
from
e074510
to
9dbf6d8
Compare
There was a problem hiding this comment.
Really great work! 👏🏻 🎉
I've left a bunch of comments that I don't consider blocking, but that I care enough about that I'd like them to be considered.
Overall I have tried to find test cases that could be missing, but after more careful consideration, I believe the scenarios I thought of are already covered by the current test suite, or are very tied to the usage of the async/await syntax itself, and are very likely already covered by goja's test suite. I ran a few test scripts on my side too and couldn't break it, so kudos! 👏🏻
🙇🏻
This is implementation of http.request, but it returns a promise and does all the networking off the event loop. The code is pretty simple and seems fairly safe given that most possible race conditions would also happen in the case of `http.batch`. Even with that there is at least one test that previously couldn't have happened with http.batch. closes #2825
mstoykov
force-pushed
the
httpAsyncRequest
branch
from
cab3a4e
to
0e5ffaa
Compare
| // Remove the trace ID from the VU's state, so that it doesn't leak into other requests. | ||
| defer func() { | ||
| c.vu.State().Tags.Modify(func(t *metrics.TagsAndMeta) { | ||
| t.DeleteMetadata(metadataTraceIDKeyName) |
There was a problem hiding this comment.
If I remove this line nothing fails for the http and tracing module's tests.
There was a problem hiding this comment.
I do think this should be addressed separately. After taking a look this is not new I just refactor the code - if anything my code is less likely to break :P. But writing a test for this seems a bit more involved than I would prefer for this PR
There was a problem hiding this comment.
I did open an issue with it #2889
Co-authored-by: Ivan <2103732+codebien@users.noreply.github.com>
mstoykov
force-pushed
the
httpAsyncRequest
branch
from
99eab43
to
8aeed54
Compare
This is implementation of http.request but it return a promise and does all the networking off the event loop.
The code is pretty simple and seems fairly safe given that most possible race conditions would also happen in the case of
http.batch.Even with that there is at least one test that previously couldn't have happened with http.batch.
closes #2825