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

Fetch Fixes #49

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3ccefb2
fix fetch attempt
nicoburniske Apr 30, 2024
2d29ef5
fix fetch test by locking stream
nicoburniske Apr 30, 2024
d2636ec
enable all tests again
nicoburniske Apr 30, 2024
0473061
remove obsolete comment
nicoburniske Apr 30, 2024
fe217ce
fetch test body ReadableStream
nicoburniske Apr 30, 2024
98bf330
add post test (failing)
nicoburniske Apr 30, 2024
ba679d5
comment out headers only body post test
nicoburniske Apr 30, 2024
7975f3b
debugging fetch-3
nicoburniske May 1, 2024
6347825
add header logs
nicoburniske May 8, 2024
9bdfce6
Merge remote-tracking branch 'origin/main' into fetch-fix-updated
nicoburniske May 8, 2024
2e883a7
Debug and fix attempts
vigoo May 13, 2024
b509286
attempt to extract value before subscribing
nicoburniske May 13, 2024
a1df1ba
update fetch-3 unit test
nicoburniske May 13, 2024
0de1f11
More debug and fixes
vigoo May 13, 2024
ef48a6e
add async stream fetch test 4
nicoburniske May 20, 2024
383f138
remove debug stderror logs
nicoburniske May 20, 2024
717c191
Merge pull request #1 from nicoburniske/fetch-fix-updated
nicoburniske May 20, 2024
0ee4e92
Merge branch 'bytecodealliance:main' into main
nicoburniske May 20, 2024
b101905
small clean up
nicoburniske May 20, 2024
f666d6d
clean up
nicoburniske May 20, 2024
76f5f40
small cleanup
nicoburniske May 20, 2024
c9b42e4
small cleanup
nicoburniske May 20, 2024
62ea77d
more cleanup
nicoburniske May 20, 2024
d0bb3bf
remove fetch-4 test
nicoburniske May 20, 2024
f7aacf1
remove clean_test.sh
nicoburniske May 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions builtins/web/fetch/request-response.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,13 @@ bool RequestOrResponse::body_unusable(JSContext *cx, JS::HandleObject body) {
return disturbed || locked;
}

bool RequestOrResponse::body_disturbed(JSContext *cx, JS::HandleObject body) {
MOZ_ASSERT(JS::IsReadableStream(body));
bool disturbed;
MOZ_RELEASE_ASSERT(JS::ReadableStreamIsDisturbed(cx, body, &disturbed));
return disturbed;
}

/**
* Implementation of the `extract a body` algorithm at
* https://fetch.spec.whatwg.org/#concept-bodyinit-extract
Expand Down Expand Up @@ -416,6 +423,8 @@ bool RequestOrResponse::extract_body(JSContext *cx, JS::HandleObject self,
auto body = RequestOrResponse::outgoing_body_handle(self);
auto write_res = body->write_all(reinterpret_cast<uint8_t *>(buf), length);

body->close();

// Ensure that the NoGC is reset, so throwing an error in HANDLE_ERROR
// succeeds.
if (maybeNoGC.isSome()) {
Expand All @@ -431,6 +440,9 @@ bool RequestOrResponse::extract_body(JSContext *cx, JS::HandleObject self,
// Step 36.3 of Request constructor / 8.4 of Response constructor.
if (content_type) {
JS::RootedObject headers(cx, RequestOrResponse::headers(cx, self));
if (!headers) {
return false;
}
if (!Headers::maybe_add(cx, headers, "content-type", content_type)) {
return false;
}
Expand Down Expand Up @@ -751,7 +763,7 @@ bool RequestOrResponse::consume_content_stream_for_bodyAll(JSContext *cx, JS::Ha
return false;
}
MOZ_ASSERT(JS::IsReadableStream(stream));
if (RequestOrResponse::body_unusable(cx, stream)) {
if (RequestOrResponse::body_disturbed(cx, stream)) {
JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
JSMSG_RESPONSE_BODY_DISTURBED_OR_LOCKED);
JS::RootedObject result_promise(cx);
Expand All @@ -760,8 +772,9 @@ bool RequestOrResponse::consume_content_stream_for_bodyAll(JSContext *cx, JS::Ha
JS::SetReservedSlot(self, static_cast<uint32_t>(Slots::BodyAllPromise), JS::UndefinedValue());
return RejectPromiseWithPendingError(cx, result_promise);
}
JS::Rooted<JSObject *> unwrappedReader(
cx, JS::ReadableStreamGetReader(cx, stream, JS::ReadableStreamReaderMode::Default));

JS::RootedObject unwrappedReader(cx, streams::NativeStreamSource::get_locked_by_internal_reader(cx, stream));

if (!unwrappedReader) {
return false;
}
Expand Down Expand Up @@ -1135,7 +1148,11 @@ JSObject *RequestOrResponse::create_body_stream(JSContext *cx, JS::HandleObject
return nullptr;
}

// TODO: immediately lock the stream if the owner's body is already used.
// If the body has already been used without being reified as a ReadableStream,
// lock the stream immediately.
if (body_used(owner)) {
MOZ_RELEASE_ASSERT(streams::NativeStreamSource::lock_stream(cx, body_stream));
}

JS_SetReservedSlot(owner, static_cast<uint32_t>(Slots::BodyStream),
JS::ObjectValue(*body_stream));
Expand Down Expand Up @@ -1693,7 +1710,8 @@ JSObject *Request::create(JSContext *cx, JS::HandleObject requestInstance, JS::H
// `init["headers"]` exists, create the request's `headers` from that,
// otherwise create it from the `init` object's `headers`, or create a new,
// empty one.
auto *headers_handle = new host_api::HttpHeaders();
auto *headers_handle = new host_api::HttpHeaders();

JS::RootedObject headers(cx);

if (headers_val.isUndefined() && input_headers) {
Expand Down
1 change: 1 addition & 0 deletions builtins/web/fetch/request-response.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class RequestOrResponse final {
static JS::Value url(JSObject *obj);
static void set_url(JSObject *obj, JS::Value url);
static bool body_unusable(JSContext *cx, JS::HandleObject body);
static bool body_disturbed(JSContext *cx, JS::HandleObject body);
static bool extract_body(JSContext *cx, JS::HandleObject self, JS::HandleValue body_val);

/**
Expand Down
13 changes: 13 additions & 0 deletions builtins/web/streams/native-stream-source.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,19 @@ bool NativeStreamSource::lock_stream(JSContext *cx, JS::HandleObject stream) {
return true;
}

JSObject *NativeStreamSource::get_locked_by_internal_reader(JSContext *cx, JS::HandleObject stream) {

JS::RootedObject self(cx, get_stream_source(cx, stream));
MOZ_ASSERT(is_instance(self));

bool locked;
JS::ReadableStreamIsLocked(cx, stream, &locked);
MOZ_ASSERT(locked);

JS::RootedObject reader(cx, &JS::GetReservedSlot(self, Slots::InternalReader).toObject());
return reader;
}

bool NativeStreamSource::start(JSContext *cx, unsigned argc, JS::Value *vp) {
METHOD_HEADER(1)

Expand Down
1 change: 1 addition & 0 deletions builtins/web/streams/native-stream-source.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class NativeStreamSource : public BuiltinNoConstructor<NativeStreamSource> {
JS::HandleObject writable);
static JSObject *piped_to_transform_stream(JSObject *self);
static bool lock_stream(JSContext *cx, JS::HandleObject stream);
static JSObject *get_locked_by_internal_reader(JSContext *cx, JS::HandleObject stream);
static bool start(JSContext *cx, unsigned argc, JS::Value *vp);
static bool pull(JSContext *cx, unsigned argc, JS::Value *vp);
static bool cancel(JSContext *cx, unsigned argc, JS::Value *vp);
Expand Down
19 changes: 14 additions & 5 deletions host-apis/wasi-0.2.0/host_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -709,21 +709,24 @@ HttpOutgoingRequest *HttpOutgoingRequest::make(string_view method_str, optional<
auto *state = new HandleState(handle.__handle);
auto *resp = new HttpOutgoingRequest(state);

resp->headers_ = headers;

return resp;
}

Result<string_view> HttpOutgoingRequest::method() {
MOZ_ASSERT(valid());
MOZ_ASSERT(headers_);
return Result<string_view>::ok(method_);
}

Result<HttpHeaders *> HttpOutgoingRequest::headers() {
typedef Result<HttpHeaders *> Res;
MOZ_ASSERT(valid());
MOZ_ASSERT(headers_);
return Result<HttpHeaders *>::ok(headers_);

if (!this->headers_) {
auto res = wasi_http_0_2_0_types_method_outgoing_request_headers(
wasi_http_0_2_0_types_borrow_outgoing_request({handle_state_->handle}));
headers_ = new HttpHeaders(res.__handle);
}
return Res::ok(this->headers_);
}

Result<HttpOutgoingBody *> HttpOutgoingRequest::body() {
Expand All @@ -742,6 +745,12 @@ Result<HttpOutgoingBody *> HttpOutgoingRequest::body() {

Result<FutureHttpIncomingResponse *> HttpOutgoingRequest::send() {
MOZ_ASSERT(valid());

if (this->headers_) {
wasi_http_0_2_0_types_fields_drop_borrow({this->headers_->handle_state_->handle});
}


future_incoming_response_t ret;
wasi_http_0_2_0_outgoing_handler_error_code_t err;
wasi_http_0_2_0_outgoing_handler_handle({handle_state_->handle}, nullptr, &ret, &err);
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/fetch-1/expect_serve_body.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[{"id":1,"name":"Leanne Graham","username":"Bret","email":"[email protected]","address":{"street":"Kulas Light","suite":"Apt. 556","city":"Gwenborough","zipcode":"92998-3874","geo":{"lat":"-37.3159","lng":"81.1496"}},"phone":"1-770-736-8031 x56442","website":"hildegard.org","company":{"name":"Romaguera-Crona","catchPhrase":"Multi-layered client-server neural-net","bs":"harness real-time e-markets"}},{"id":2,"name":"Ervin Howell","username":"Antonette","email":"[email protected]","address":{"street":"Victor Plains","suite":"Suite 879","city":"Wisokyburgh","zipcode":"90566-7771","geo":{"lat":"-43.9509","lng":"-34.4618"}},"phone":"010-692-6593 x09125","website":"anastasia.net","company":{"name":"Deckow-Crist","catchPhrase":"Proactive didactic contingency","bs":"synergize scalable supply-chains"}},{"id":3,"name":"Clementine Bauch","username":"Samantha","email":"[email protected]","address":{"street":"Douglas Extension","suite":"Suite 847","city":"McKenziehaven","zipcode":"59590-4157","geo":{"lat":"-68.6102","lng":"-47.0653"}},"phone":"1-463-123-4447","website":"ramiro.info","company":{"name":"Romaguera-Jacobson","catchPhrase":"Face to face bifurcated interface","bs":"e-enable strategic applications"}},{"id":4,"name":"Patricia Lebsack","username":"Karianne","email":"[email protected]","address":{"street":"Hoeger Mall","suite":"Apt. 692","city":"South Elvis","zipcode":"53919-4257","geo":{"lat":"29.4572","lng":"-164.2990"}},"phone":"493-170-9623 x156","website":"kale.biz","company":{"name":"Robel-Corkery","catchPhrase":"Multi-tiered zero tolerance productivity","bs":"transition cutting-edge web services"}},{"id":5,"name":"Chelsey Dietrich","username":"Kamren","email":"[email protected]","address":{"street":"Skiles Walks","suite":"Suite 351","city":"Roscoeview","zipcode":"33263","geo":{"lat":"-31.8129","lng":"62.5342"}},"phone":"(254)954-1289","website":"demarco.info","company":{"name":"Keebler LLC","catchPhrase":"User-centric fault-tolerant solution","bs":"revolutionize end-to-end systems"}},{"id":6,"name":"Mrs. Dennis Schulist","username":"Leopoldo_Corkery","email":"[email protected]","address":{"street":"Norberto Crossing","suite":"Apt. 950","city":"South Christy","zipcode":"23505-1337","geo":{"lat":"-71.4197","lng":"71.7478"}},"phone":"1-477-935-8478 x6430","website":"ola.org","company":{"name":"Considine-Lockman","catchPhrase":"Synchronised bottom-line interface","bs":"e-enable innovative applications"}},{"id":7,"name":"Kurtis Weissnat","username":"Elwyn.Skiles","email":"[email protected]","address":{"street":"Rex Trail","suite":"Suite 280","city":"Howemouth","zipcode":"58804-1099","geo":{"lat":"24.8918","lng":"21.8984"}},"phone":"210.067.6132","website":"elvis.io","company":{"name":"Johns Group","catchPhrase":"Configurable multimedia task-force","bs":"generate enterprise e-tailers"}},{"id":8,"name":"Nicholas Runolfsdottir V","username":"Maxime_Nienow","email":"[email protected]","address":{"street":"Ellsworth Summit","suite":"Suite 729","city":"Aliyaview","zipcode":"45169","geo":{"lat":"-14.3990","lng":"-120.7677"}},"phone":"586.493.6943 x140","website":"jacynthe.com","company":{"name":"Abernathy Group","catchPhrase":"Implemented secondary concept","bs":"e-enable extensible e-tailers"}},{"id":9,"name":"Glenna Reichert","username":"Delphine","email":"[email protected]","address":{"street":"Dayna Park","suite":"Suite 449","city":"Bartholomebury","zipcode":"76495-3109","geo":{"lat":"24.6463","lng":"-168.8889"}},"phone":"(775)976-6794 x41206","website":"conrad.com","company":{"name":"Yost and Sons","catchPhrase":"Switchable contextually-based project","bs":"aggregate real-time technologies"}},{"id":10,"name":"Clementina DuBuque","username":"Moriah.Stanton","email":"[email protected]","address":{"street":"Kattie Turnpike","suite":"Suite 198","city":"Lebsackbury","zipcode":"31428-2261","geo":{"lat":"-38.2386","lng":"57.2232"}},"phone":"024-648-3804","website":"ambrose.net","company":{"name":"Hoeger LLC","catchPhrase":"Centralized empowering task-force","bs":"target end-to-end models"}}]
Empty file.
1 change: 1 addition & 0 deletions tests/e2e/fetch-1/expect_serve_stdout.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
stdout [0] :: Log: Successfully received response json body
22 changes: 22 additions & 0 deletions tests/e2e/fetch-1/fetch-1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
async function main(event) {
let resolve, reject;

try {
let responsePromise = new Promise(async (res, rej) => {
resolve = res;
reject = rej;
});
event.respondWith(responsePromise);

let response = await fetch("https://jsonplaceholder.typicode.com/users");
let text = await response.json();

console.log("Successfully received response json body");

resolve(new Response(JSON.stringify(text), { headers: response.headers }));
} catch (e) {
console.log(`Error: ${e}. Stack: ${e.stack}`);
}
}

addEventListener('fetch', main);
Loading
Loading