Request Duplex (Readable Stream) Support

[mookums]

This adds support for Request taking a ReadableStream.

https://developer.mozilla.org/en-US/docs/Web/API/Request/duplex

[krichprollsch]

@mookums the PR is in draft, is it ready for review?

[mookums]

Was mostly finished but needed some updating on some tests.

[krichprollsch]

@mookums are you still working on this one?

[mookums]

Yeah, done now. It supports taking in a ReadableStream for the Request body, consuming it before use and throwing an error if it isn't ready. It prevents the error on #3163 that is present on main.

[krichprollsch]

One suggestion to resolve the sharing stream issue could be:

  // Resolve a stream body to bytes on first need, so every copy (clone(), or
  // `new Request(other)`) owns its own bytes. Null means no body.
  fn resolveBody(self: *Request) !?[]const u8 {
      switch (self._body) {
          .none => return null,
          .bytes => |b| return b,
          .stream => |s| {
              const collected = s.collectBodyBytes(self._arena.allocator()) catch return error.TypeError;
              self._body = .{ .bytes = collected };
              return collected;
          },
      }
  }

Then:

  pub fn clone(self: *Request, exec: *const Execution) !*Request {
      // Fetch: clone() throws if the body is already disturbed.
      if (self._body_used) return error.TypeError;

      const body = try self.resolveBody();
      const arena = try exec.getPinnedArena(if (body) |b| b.len else 512, "Request.clone");
      errdefer arena.release();
      ...
      ._body = if (body) |b| .{ .bytes = try arena.dupe(u8, b) } else .none,

and

  .request => |r| blk: {
      if (r._body_used) return error.TypeError;
      break :blk if (try r.resolveBody()) |b| .{ .bytes = try arena.dupe(u8, b) } else .none;
  },

Or we need Tee for request clone

[krichprollsch]

Shouldn't we keep this early return by testing the case of body == .none?

[krichprollsch]

Keep the hint when body is bytes.

  const arena = try exec.getPinnedArena(switch (self._body) {
      .bytes => |b| b.len,
      .none, .stream => 512,
  }, "Request.clone");

[krichprollsch]

You drain the stream, but you let the request in an invalid state.
you should replace the request's body with the collected bytes.
as you do in Request.consume

[krichprollsch]

Not sure it's safe to share the stream. If one collect it, the other won't be updated 🤔

[krichprollsch]

same about sharing the stream