Skip to content

refactor(Response) isolate body usage #23313

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Oct 7, 2025
Merged

refactor(Response) isolate body usage #23313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b21a208
isolate Response fields
cirospaciari Oct 7, 2025
368ba17
[autofix.ci] apply automated fixes
autofix-ci[bot] Oct 7, 2025
dde97af
check if promise is already handled before rejecting
cirospaciari Oct 7, 2025
d98eb1f
fix(Response) change owner ship of the stream to allow cyclic referen…
cirospaciari Oct 7, 2025
5cc5bfe
Simplify
Jarred-Sumner Oct 7, 2025
e0e7ce2
Update RequestContext.zig
Jarred-Sumner Oct 7, 2025
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
10 changes: 6 additions & 4 deletions src/bake/DevServer.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3353,13 +3353,15 @@ fn sendSerializedFailures(
}
}
const fetch_headers = try headers.toFetchHeaders(r.global);
var response = Response{
.body = .{ .value = .{ .Blob = any_blob.toBlob(r.global) } },
.init = Response.Init{
var response = Response.init(
.{
.status_code = 500,
.headers = fetch_headers,
},
};
.{ .value = .{ .Blob = any_blob.toBlob(r.global) } },
bun.String.empty,
false,
);
dev.vm.eventLoop().enter();
r.promise.reject(r.global, response.toJS(r.global));
defer dev.vm.eventLoop().exit();
Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/api/filesystem_router.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ pub const FileSystemRouter = struct {
}

if (argument.as(jsc.WebCore.Response)) |resp| {
break :brk resp.url.toUTF8(globalThis.allocator());
break :brk resp.getUTF8Url(globalThis.allocator());
}
}

Expand Down
74 changes: 42 additions & 32 deletions src/bun.js/api/html_rewriter.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ pub const HTMLRewriter = struct {

pub fn transform_(this: *HTMLRewriter, global: *JSGlobalObject, response_value: jsc.JSValue) bun.JSError!JSValue {
if (response_value.as(Response)) |response| {
if (response.body.value == .Used) {
const body_value = response.getBodyValue();
if (body_value.* == .Used) {
return global.throwInvalidArguments("Response body already used", .{});
}
const out = try this.beginTransform(global, response);
Expand All @@ -198,12 +199,14 @@ pub const HTMLRewriter = struct {
if (kind != .other) {
{
const body_value = try jsc.WebCore.Body.extract(global, response_value);
const resp = bun.new(Response, Response{
.init = .{
const resp = bun.new(Response, Response.init(
.{
.status_code = 200,
},
.body = body_value,
});
body_value,
bun.String.empty,
false,
));
defer resp.finalize();
const out_response_value = try this.beginTransform(global, resp);
// Check if the returned value is an error and throw it properly
Expand All @@ -212,7 +215,7 @@ pub const HTMLRewriter = struct {
}
out_response_value.ensureStillAlive();
var out_response = out_response_value.as(Response) orelse return out_response_value;
var blob = out_response.body.value.useAsAnyBlobAllowNonUTF8String();
var blob = out_response.getBodyValue().useAsAnyBlobAllowNonUTF8String();

defer {
_ = Response.js.dangerouslySetPtr(out_response_value, null);
Expand Down Expand Up @@ -415,23 +418,25 @@ pub const HTMLRewriter = struct {
.response = undefined,
});
defer sink.deref();
var result = bun.new(Response, .{
.init = .{
var result = bun.new(Response, Response.init(
.{
.status_code = 200,
},
.body = .{
.{
.value = .{
.Locked = .{
.global = global,
.task = sink,
},
},
},
});
bun.String.empty,
false,
));

sink.response = result;
var sink_error: jsc.JSValue = .zero;
const input_size = original.body.len();
const input_size = original.getBodyLen();
var vm = global.bunVM();

// Since we're still using vm.waitForPromise, we have to also
Expand Down Expand Up @@ -467,27 +472,30 @@ pub const HTMLRewriter = struct {
return createLOLHTMLError(global);
};

result.init.method = original.init.method;
result.init.status_code = original.init.status_code;
result.init.status_text = original.init.status_text.clone();
result.setInit(
original.getMethod(),
original.getInitStatusCode(),
original.getInitStatusText().clone(),
);

// https://github.com/oven-sh/bun/issues/3334
if (original.init.headers) |headers| {
result.init.headers = try headers.cloneThis(global);
if (original.getInitHeaders()) |_headers| {
result.setInitHeaders(try _headers.cloneThis(global));
}

// Hold off on cloning until we're actually done.
const response_js_value = sink.response.toJS(sink.global);
sink.response_value.set(global, response_js_value);

result.url = original.url.clone();
result.setUrl(original.getUrl().clone());

const value = original.getBodyValue();
const owned_readable_stream = original.getBodyReadableStream(sink.global);
sink.ref();
sink.bodyValueBufferer = jsc.WebCore.Body.ValueBufferer.init(sink, @ptrCast(&onFinishedBuffering), sink.global, bun.default_allocator);
response_js_value.ensureStillAlive();

sink.bodyValueBufferer.?.run(value) catch |buffering_error| {
sink.bodyValueBufferer.?.run(value, owned_readable_stream) catch |buffering_error| {
defer sink.deref();
return switch (buffering_error) {
error.StreamAlreadyUsed => {
Expand Down Expand Up @@ -522,21 +530,22 @@ pub const HTMLRewriter = struct {
pub fn onFinishedBuffering(sink: *BufferOutputSink, bytes: []const u8, js_err: ?jsc.WebCore.Body.Value.ValueError, is_async: bool) void {
defer sink.deref();
if (js_err) |err| {
if (sink.response.body.value == .Locked and @intFromPtr(sink.response.body.value.Locked.task) == @intFromPtr(sink) and
sink.response.body.value.Locked.promise == null)
const sinkBodyValue = sink.response.getBodyValue();
if (sinkBodyValue.* == .Locked and @intFromPtr(sinkBodyValue.Locked.task) == @intFromPtr(sink) and
sinkBodyValue.Locked.promise == null)
{
sink.response.body.value.Locked.readable.deinit();
sink.response.body.value = .{ .Empty = {} };
sinkBodyValue.Locked.readable.deinit();
sinkBodyValue.* = .{ .Empty = {} };
// is there a pending promise?
// we will need to reject it
} else if (sink.response.body.value == .Locked and @intFromPtr(sink.response.body.value.Locked.task) == @intFromPtr(sink) and
sink.response.body.value.Locked.promise != null)
} else if (sinkBodyValue.* == .Locked and @intFromPtr(sinkBodyValue.Locked.task) == @intFromPtr(sink) and
sinkBodyValue.Locked.promise != null)
{
sink.response.body.value.Locked.onReceiveValue = null;
sink.response.body.value.Locked.task = null;
sinkBodyValue.Locked.onReceiveValue = null;
sinkBodyValue.Locked.task = null;
}
if (is_async) {
sink.response.body.value.toErrorInstance(err.dupe(sink.global), sink.global);
sinkBodyValue.toErrorInstance(err.dupe(sink.global), sink.global);
} else {
var ret_err = createLOLHTMLError(sink.global);
ret_err.ensureStillAlive();
Expand Down Expand Up @@ -566,7 +575,7 @@ pub const HTMLRewriter = struct {

sink.rewriter.?.write(bytes) catch {
if (is_async) {
response.body.value.toErrorInstance(.{ .Message = createLOLHTMLStringError() }, global);
response.getBodyValue().toErrorInstance(.{ .Message = createLOLHTMLStringError() }, global);
return null;
} else {
return createLOLHTMLError(global);
Expand All @@ -577,7 +586,7 @@ pub const HTMLRewriter = struct {
if (!is_async) response.finalize();
sink.response = undefined;
if (is_async) {
response.body.value.toErrorInstance(.{ .Message = createLOLHTMLStringError() }, global);
response.getBodyValue().toErrorInstance(.{ .Message = createLOLHTMLStringError() }, global);
return null;
} else {
return createLOLHTMLError(global);
Expand All @@ -590,8 +599,9 @@ pub const HTMLRewriter = struct {
pub const Sync = enum { suspended, pending, done };

pub fn done(this: *BufferOutputSink) void {
var prev_value = this.response.body.value;
this.response.body.value = jsc.WebCore.Body.Value{
const bodyValue = this.response.getBodyValue();
var prev_value = bodyValue.*;
bodyValue.* = jsc.WebCore.Body.Value{
.InternalBlob = .{
.bytes = this.bytes.list.toManaged(bun.default_allocator),
},
Expand All @@ -606,7 +616,7 @@ pub const HTMLRewriter = struct {
};

prev_value.resolve(
&this.response.body.value,
bodyValue,
this.global,
null,
);
Expand Down
32 changes: 16 additions & 16 deletions src/bun.js/api/server.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1186,7 +1186,7 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
}
}

existing_request = Request.init(
existing_request = Request.init2(
bun.String.cloneUTF8(url.href),
headers,
bun.handleOom(this.vm.initRequestBodyValue(body)),
Expand Down Expand Up @@ -1228,7 +1228,7 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
}

if (response_value.as(jsc.WebCore.Response)) |resp| {
resp.url = existing_request.url.clone();
resp.setUrl(existing_request.url.clone());
}
return jsc.JSPromise.resolvedPromiseValue(ctx, response_value);
}
Expand Down Expand Up @@ -2213,13 +2213,13 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
ctx.signal = signal;
signal.pendingActivityRef();

const request_object = Request.new(.{
.method = ctx.method,
.request_context = AnyRequestContext.init(ctx),
.https = ssl_enabled,
.signal = signal.ref(),
.body = body.ref(),
});
const request_object = Request.new(Request.init(
ctx.method,
AnyRequestContext.init(ctx),
ssl_enabled,
signal.ref(),
body.ref(),
));
ctx.request_weakref = .initRef(request_object);

if (comptime debug_mode) {
Expand Down Expand Up @@ -2314,13 +2314,13 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
var signal = jsc.WebCore.AbortSignal.new(this.globalThis);
ctx.signal = signal;

var request_object = Request.new(.{
.method = ctx.method,
.request_context = AnyRequestContext.init(ctx),
.https = ssl_enabled,
.signal = signal.ref(),
.body = body.ref(),
});
var request_object = Request.new(Request.init(
ctx.method,
AnyRequestContext.init(ctx),
ssl_enabled,
signal.ref(),
body.ref(),
));
ctx.upgrade_context = upgrade_ctx;
ctx.request_weakref = .initRef(request_object);
// We keep the Request object alive for the duration of the request so that we can remove the pointer to the UWS request object.
Expand Down
13 changes: 7 additions & 6 deletions src/bun.js/api/server/FileRoute.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,19 @@ pub fn memoryCost(this: *const FileRoute) usize {

pub fn fromJS(globalThis: *jsc.JSGlobalObject, argument: jsc.JSValue) bun.JSError!?*FileRoute {
if (argument.as(jsc.WebCore.Response)) |response| {
response.body.value.toBlobIfPossible();
if (response.body.value == .Blob and response.body.value.Blob.needsToReadFile()) {
if (response.body.value.Blob.store.?.data.file.pathlike == .fd) {
const bodyValue = response.getBodyValue();
bodyValue.toBlobIfPossible();
if (bodyValue.* == .Blob and bodyValue.Blob.needsToReadFile()) {
if (bodyValue.Blob.store.?.data.file.pathlike == .fd) {
return globalThis.throwTODO("Support serving files from a file descriptor. Please pass a path instead.");
}

var blob = response.body.value.use();
var blob = bodyValue.use();

blob.globalThis = globalThis;
bun.assertf(!blob.isHeapAllocated(), "expected blob not to be heap-allocated", .{});
response.body.value = .{ .Blob = blob.dupe() };
const headers = bun.handleOom(Headers.from(response.init.headers, bun.default_allocator, .{ .body = &.{ .Blob = blob } }));
bodyValue.* = .{ .Blob = blob.dupe() };
const headers = bun.handleOom(Headers.from(response.getInitHeaders(), bun.default_allocator, .{ .body = &.{ .Blob = blob } }));

return bun.new(FileRoute, .{
.ref_count = .init(),
Expand Down
Loading