From 25b91e5c866aeb1df4841e9b2a22ac2fc57fcd91 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Sun, 23 Nov 2025 00:32:38 -0800 Subject: [PATCH] Update `JSValue.toSliceClone` to use `JSError` (#24949) ### What does this PR do? Removes a TODO ### How did you verify your code works? --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- src/bun.js/api/JSBundler.zig | 4 ++-- src/bun.js/api/JSTranspiler.zig | 2 +- src/bun.js/api/glob.zig | 2 +- src/bun.js/bindings/JSValue.zig | 20 ++++---------------- src/bun.js/webcore/Blob.zig | 9 ++++++--- src/bun.js/webcore/fetch.zig | 4 +--- src/install/PackageManager/UpdateRequest.zig | 8 +++----- test/internal/ban-limits.json | 4 ++-- 8 files changed, 20 insertions(+), 33 deletions(-) diff --git a/src/bun.js/api/JSBundler.zig b/src/bun.js/api/JSBundler.zig index dfbf1adc9e..0767c6e218 100644 --- a/src/bun.js/api/JSBundler.zig +++ b/src/bun.js/api/JSBundler.zig @@ -946,8 +946,8 @@ pub const JSBundler = struct { resolve.value = .{ .no_match = {} }; } else { const global = resolve.bv2.plugins.?.globalObject(); - const path = path_value.toSliceCloneWithAllocator(global, bun.default_allocator) orelse @panic("Unexpected: path is not a string"); - const namespace = namespace_value.toSliceCloneWithAllocator(global, bun.default_allocator) orelse @panic("Unexpected: namespace is not a string"); + const path = path_value.toSliceCloneWithAllocator(global, bun.default_allocator) catch @panic("Unexpected: path is not a string"); + const namespace = namespace_value.toSliceCloneWithAllocator(global, bun.default_allocator) catch @panic("Unexpected: namespace is not a string"); resolve.value = .{ .success = .{ .path = path.slice(), diff --git a/src/bun.js/api/JSTranspiler.zig b/src/bun.js/api/JSTranspiler.zig index d5f004113f..6a77dc0efd 100644 --- a/src/bun.js/api/JSTranspiler.zig +++ b/src/bun.js/api/JSTranspiler.zig @@ -388,7 +388,7 @@ pub const Config = struct { const replacementValue = try value.getIndex(globalThis, 1); if (try exportReplacementValue(replacementValue, globalThis, allocator)) |to_replace| { const replacementKey = try value.getIndex(globalThis, 0); - var slice = replacementKey.toSliceCloneWithAllocator(globalThis, allocator) orelse return error.JSError; + var slice = try replacementKey.toSliceCloneWithAllocator(globalThis, allocator); errdefer slice.deinit(); const replacement_name = slice.slice(); diff --git a/src/bun.js/api/glob.zig b/src/bun.js/api/glob.zig index 6f66f6b4ec..c9dcac5cf3 100644 --- a/src/bun.js/api/glob.zig +++ b/src/bun.js/api/glob.zig @@ -275,7 +275,7 @@ pub fn constructor(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) b return globalThis.throw("Glob.constructor: first argument is not a string", .{}); } - const pat_str: []u8 = @constCast((pat_arg.toSliceClone(globalThis) orelse return error.JSError).slice()); + const pat_str: []u8 = @constCast((try pat_arg.toSliceClone(globalThis)).slice()); const glob = bun.handleOom(alloc.create(Glob)); glob.* = .{ .pattern = pat_str }; diff --git a/src/bun.js/bindings/JSValue.zig b/src/bun.js/bindings/JSValue.zig index 75a891f659..77b7daa36d 100644 --- a/src/bun.js/bindings/JSValue.zig +++ b/src/bun.js/bindings/JSValue.zig @@ -1219,30 +1219,18 @@ pub const JSValue = enum(i64) { /// On exception or out of memory, this returns null. /// /// Remember that `Symbol` throws an exception when you call `toString()`. - pub fn toSliceClone(this: JSValue, globalThis: *JSGlobalObject) ?ZigString.Slice { + pub fn toSliceClone(this: JSValue, globalThis: *JSGlobalObject) bun.JSError!ZigString.Slice { return this.toSliceCloneWithAllocator(globalThis, bun.default_allocator); } - /// Call `toString()` on the JSValue and clone the result. - /// On exception or out of memory, this returns null. - /// - /// Remember that `Symbol` throws an exception when you call `toString()`. - pub fn toSliceCloneZ(this: JSValue, globalThis: *JSGlobalObject) JSError!?[:0]u8 { - var str = try bun.String.fromJS(this, globalThis); - return try str.toOwnedSliceZ(bun.default_allocator); - } - /// On exception or out of memory, this returns null, to make exception checks clearer. pub fn toSliceCloneWithAllocator( this: JSValue, globalThis: *JSGlobalObject, allocator: std.mem.Allocator, - ) ?ZigString.Slice { - var str = this.toJSString(globalThis) catch return null; - return str.toSliceClone(globalThis, allocator) catch { - globalThis.throwOutOfMemory() catch {}; // TODO: properly propagate exception upwards - return null; - }; + ) JSError!ZigString.Slice { + var str = try this.toJSString(globalThis); + return str.toSliceClone(globalThis, allocator); } /// Runtime conversion to an object. This can have side effects. diff --git a/src/bun.js/webcore/Blob.zig b/src/bun.js/webcore/Blob.zig index 90d6b452ea..0ec23eb690 100644 --- a/src/bun.js/webcore/Blob.zig +++ b/src/bun.js/webcore/Blob.zig @@ -3859,7 +3859,8 @@ fn fromJSWithoutDeferGC( } else { return build.blob.dupe(); } - } else if (current.toSliceClone(global)) |sliced| { + } else { + const sliced = try current.toSliceClone(global); if (sliced.allocator.get()) |allocator| { return Blob.initWithAllASCII(@constCast(sliced.slice()), allocator, global, false); } @@ -3955,7 +3956,8 @@ fn fromJSWithoutDeferGC( could_have_non_ascii = could_have_non_ascii or blob.charset != .all_ascii; joiner.pushStatic(blob.sharedView()); continue; - } else if (current.toSliceClone(global)) |sliced| { + } else { + const sliced = try current.toSliceClone(global); const allocator = sliced.allocator.get(); could_have_non_ascii = could_have_non_ascii or allocator != null; joiner.push(sliced.slice(), allocator); @@ -3973,7 +3975,8 @@ fn fromJSWithoutDeferGC( if (current.as(Blob)) |blob| { could_have_non_ascii = could_have_non_ascii or blob.charset != .all_ascii; joiner.pushStatic(blob.sharedView()); - } else if (current.toSliceClone(global)) |sliced| { + } else { + const sliced = try current.toSliceClone(global); const allocator = sliced.allocator.get(); could_have_non_ascii = could_have_non_ascii or allocator != null; joiner.push(sliced.slice(), allocator); diff --git a/src/bun.js/webcore/fetch.zig b/src/bun.js/webcore/fetch.zig index 9a197caf9f..149639ce71 100644 --- a/src/bun.js/webcore/fetch.zig +++ b/src/bun.js/webcore/fetch.zig @@ -490,9 +490,7 @@ pub fn Bun__fetch_( if (objects_to_try[i] != .zero) { if (try objects_to_try[i].get(globalThis, "unix")) |socket_path| { if (socket_path.isString() and try socket_path.getLength(ctx) > 0) { - if (socket_path.toSliceCloneWithAllocator(globalThis, allocator)) |slice| { - break :extract_unix_socket_path slice; - } + break :extract_unix_socket_path try socket_path.toSliceCloneWithAllocator(globalThis, allocator); } } diff --git a/src/install/PackageManager/UpdateRequest.zig b/src/install/PackageManager/UpdateRequest.zig index 0b9f5dd9fe..eb9deb919e 100644 --- a/src/install/PackageManager/UpdateRequest.zig +++ b/src/install/PackageManager/UpdateRequest.zig @@ -56,21 +56,19 @@ pub fn fromJS(globalThis: *jsc.JSGlobalObject, input: jsc.JSValue) bun.JSError!j var log = logger.Log.init(allocator); if (input.isString()) { - var input_str = input.toSliceCloneWithAllocator( + var input_str = try input.toSliceCloneWithAllocator( globalThis, allocator, - ) orelse return .zero; + ); if (input_str.len > 0) try all_positionals.append(input_str.slice()); } else if (input.isArray()) { var iter = try input.arrayIterator(globalThis); while (try iter.next()) |item| { - const slice = item.toSliceCloneWithAllocator(globalThis, allocator) orelse return .zero; - if (globalThis.hasException()) return .zero; + const slice = try item.toSliceCloneWithAllocator(globalThis, allocator); if (slice.len == 0) continue; try all_positionals.append(slice.slice()); } - if (globalThis.hasException()) return .zero; } else { return .js_undefined; } diff --git a/test/internal/ban-limits.json b/test/internal/ban-limits.json index 82ae899b8b..3d7f316804 100644 --- a/test/internal/ban-limits.json +++ b/test/internal/ban-limits.json @@ -17,14 +17,14 @@ "EXCEPTION_ASSERT(!scope.exception())": 0, "JSValue.false": 0, "JSValue.true": 0, - "TODO: properly propagate exception upwards": 118, + "TODO: properly propagate exception upwards": 117, "alloc.ptr !=": 0, "alloc.ptr ==": 0, "allocator.ptr !=": 1, "allocator.ptr ==": 0, "global.hasException": 28, "globalObject.hasException": 47, - "globalThis.hasException": 125, + "globalThis.hasException": 123, "std.StringArrayHashMap(": 1, "std.StringArrayHashMapUnmanaged(": 11, "std.StringHashMap(": 0,