fix: empty body responses (supabase#84)

* Fix body null for null status * Post request * adding size * clippy,fmt * hasReqBody
nyannyacha · May 10, 2023 · c02987e · c02987e
1 parent 31180a0
commit c02987e
Show file tree

Hide file tree

Showing 7 changed files with 95 additions and 7 deletions.
diff --git a/crates/base/test_cases/empty-response/index.ts b/crates/base/test_cases/empty-response/index.ts
@@ -0,0 +1,5 @@
+import { serve } from "https://deno.land/std@0.131.0/http/server.ts"
+
+serve(async (req: Request) => {
+    return new Response(null, { status: 204 })
+});
diff --git a/crates/base/tests/null_body_status_null_body_tests.rs b/crates/base/tests/null_body_status_null_body_tests.rs
@@ -0,0 +1,71 @@
+use base::worker_ctx::{create_worker, WorkerRequestMsg};
+use hyper::{Body, Request, Response};
+use sb_worker_context::essentials::{EdgeContextInitOpts, EdgeContextOpts, EdgeUserRuntimeOpts};
+use std::collections::HashMap;
+use tokio::sync::oneshot;
+
+#[tokio::test]
+async fn test_null_body_with_204_status() {
+    let user_rt_opts = EdgeUserRuntimeOpts::default();
+    let opts = EdgeContextInitOpts {
+        service_path: "./test_cases/empty-response".into(),
+        no_module_cache: false,
+        import_map_path: None,
+        env_vars: HashMap::new(),
+        conf: EdgeContextOpts::UserWorker(user_rt_opts),
+    };
+    let worker_req_tx = create_worker(opts).await.unwrap();
+    let (res_tx, res_rx) = oneshot::channel::<Result<Response<Body>, hyper::Error>>();
+
+    let req = Request::builder()
+        .uri("/")
+        .method("GET")
+        .body(Body::empty())
+        .unwrap();
+
+    let msg = WorkerRequestMsg { req, res_tx };
+    let _ = worker_req_tx.send(msg);
+
+    let res = res_rx.await.unwrap().unwrap();
+    assert!(res.status().as_u16() == 204);
+
+    let body_bytes = hyper::body::to_bytes(res.into_body())
+        .await
+        .unwrap()
+        .to_vec();
+
+    assert_eq!(body_bytes.len(), 0);
+}
+
+#[tokio::test]
+async fn test_null_body_with_204_status_post() {
+    let user_rt_opts = EdgeUserRuntimeOpts::default();
+    let opts = EdgeContextInitOpts {
+        service_path: "./test_cases/empty-response".into(),
+        no_module_cache: false,
+        import_map_path: None,
+        env_vars: HashMap::new(),
+        conf: EdgeContextOpts::UserWorker(user_rt_opts),
+    };
+    let worker_req_tx = create_worker(opts).await.unwrap();
+    let (res_tx, res_rx) = oneshot::channel::<Result<Response<Body>, hyper::Error>>();
+
+    let req = Request::builder()
+        .uri("/")
+        .method("POST")
+        .body(Body::empty())
+        .unwrap();
+
+    let msg = WorkerRequestMsg { req, res_tx };
+    let _ = worker_req_tx.send(msg);
+
+    let res = res_rx.await.unwrap().unwrap();
+    assert!(res.status().as_u16() == 204);
+
+    let body_bytes = hyper::body::to_bytes(res.into_body())
+        .await
+        .unwrap()
+        .to_vec();
+
+    assert_eq!(body_bytes.len(), 0);
+}
diff --git a/crates/base/tests/oak_user_worker_tests.rs b/crates/base/tests/oak_user_worker_tests.rs
@@ -1,3 +1,4 @@
+mod null_body_status_null_body_tests;
 mod tls_invalid_data_tests;
 
 use base::worker_ctx::{create_worker, WorkerRequestMsg};

diff --git a/crates/sb_core/js/errors.js b/crates/sb_core/js/errors.js
@@ -25,6 +25,8 @@ const buildDomErrorClass = (name) => class extends DOMException {
     }
 }
 
+const InvalidWorkerResponse = buildErrorClass('InvalidWorkerResponse');
+const InvalidWorkerCreation = buildErrorClass('InvalidWorkerCreation');
 const NotFound = buildErrorClass('NotFound');
 const PermissionDenied = buildErrorClass('PermissionDenied');
 const ConnectionRefused = buildErrorClass('ConnectionRefused');
@@ -52,6 +54,8 @@ const DOMExceptionInvalidCharacterError = buildDomErrorClass('InvalidCharacterEr
 const DOMExceptionDataError = buildDomErrorClass('DOMExceptionDataError');
 
 function registerErrors() {
+    core.registerErrorClass("InvalidWorkerResponse", InvalidWorkerResponse);
+    core.registerErrorClass("InvalidWorkerCreation", InvalidWorkerCreation);
     core.registerErrorClass("NotFound", NotFound);
     core.registerErrorClass("PermissionDenied", PermissionDenied);
     core.registerErrorClass("ConnectionRefused", ConnectionRefused);

diff --git a/crates/sb_workers/lib.rs b/crates/sb_workers/lib.rs
@@ -90,7 +90,7 @@ pub async fn op_user_worker_create(
     let result = result_rx.await;
     if result.is_err() {
         return Err(custom_error(
-            "create_user_worker_error",
+            "InvalidWorkerCreation",
             "failed to create worker",
         ));
     }
@@ -99,7 +99,7 @@ pub async fn op_user_worker_create(
     let result = result.unwrap();
     if result.is_err() {
         return Err(custom_error(
-            "create_user_worker_error",
+            "InvalidWorkerCreation",
             result.unwrap_err().to_string(),
         ));
     }
@@ -129,6 +129,7 @@ pub struct UserWorkerResponse {
     status_text: String,
     headers: Vec<(ByteString, ByteString)>,
     body_rid: ResourceId,
+    size: Option<u64>,
 }
 
 struct UserWorkerRequestResource(Request<Body>);
@@ -317,7 +318,7 @@ pub async fn op_user_worker_fetch_send(
     let result = result_rx.await?;
     if result.is_err() {
         return Err(custom_error(
-            "user_worker_fetch",
+            "InvalidWorkerResponse",
             "user worker not available",
         ));
     }
@@ -358,6 +359,7 @@ pub async fn op_user_worker_fetch_send(
         status_text,
         headers,
         body_rid,
+        size,
     };
     Ok(response)
 }

diff --git a/crates/sb_workers/user_workers.js b/crates/sb_workers/user_workers.js
@@ -29,27 +29,27 @@ class UserWorker {
         const { method, url, headers, body, bodyUsed } = req;
 
         const headersArray = Array.from(headers.entries());
-        const hasBody = !bodyUsed && !!body && (chunkExpression.test(headers.get("transfer-encoding")) ||
+        const hasReqBody = !bodyUsed && !!body && (chunkExpression.test(headers.get("transfer-encoding")) ||
                         Number.parseInt(headers.get("content-length"), 10) > 0);
 
         const userWorkerReq = {
             method,
             url,
             headers: headersArray,
-            hasBody,
+            hasBody: hasReqBody,
         };
 
         const { requestRid, requestBodyRid } = await ops.op_user_worker_fetch_build(userWorkerReq);
 
         // stream the request body
-        if (hasBody) {
+        if (hasReqBody) {
             let writableStream = writableStreamForRid(requestBodyRid);
             body.pipeTo(writableStream);
         }
 
         const res = await core.opAsync("op_user_worker_fetch_send", this.key, requestRid);
         const bodyStream = readableStreamForRid(res.bodyRid);
-        return new Response(bodyStream, {
+        return new Response(res.size ? bodyStream : null, {
             headers: res.headers,
             status: res.status,
             statusText: res.statusText

diff --git a/examples/empty-response/index.ts b/examples/empty-response/index.ts
@@ -0,0 +1,5 @@
+import { serve } from "https://deno.land/std@0.131.0/http/server.ts"
+
+serve(async (req: Request) => {
+    return new Response(null, { status: 204 })
+});