Skip to content

Update JSValue.toSliceClone to use JSError#24949

Merged
Jarred-Sumner merged 2 commits into
mainfrom
dylan/to-slice-clone-js-error
Nov 23, 2025
Merged

Update JSValue.toSliceClone to use JSError#24949
Jarred-Sumner merged 2 commits into
mainfrom
dylan/to-slice-clone-js-error

Conversation

@dylan-conway

Copy link
Copy Markdown
Member

What does this PR do?

Removes a TODO

How did you verify your code works?

@robobun

robobun commented Nov 22, 2025

Copy link
Copy Markdown
Collaborator
Updated 9:14 PM PT - Nov 21st, 2025

@autofix-ci[bot], your commit ac1184d has 3 failures in Build #32223 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 24949

That installs a local version of the PR into your bun-24949 executable, so you can run:

bun-24949 --bun

@coderabbitai

coderabbitai Bot commented Nov 22, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The PR converts error handling for string conversion methods from nullable/orelse patterns to throwing error patterns in JSValue, specifically changing toSliceClone and toSliceCloneWithAllocator signatures to return JSError!ZigString.Slice instead of nullable types, with cascading updates across call sites.

Changes

Cohort / File(s) Summary
Core API Redesign
src/bun.js/bindings/JSValue.zig
Changed toSliceClone and toSliceCloneWithAllocator from returning nullable ?ZigString.Slice to throwing bun.JSError!ZigString.Slice; removed legacy toSliceCloneZ method
API Call Sites
src/bun.js/api/JSBundler.zig, src/bun.js/api/JSTranspiler.zig, src/bun.js/api/glob.zig
Updated error handling from orelse/nullable patterns to try/catch expressions for string-slice conversions
Webcore Integration
src/bun.js/webcore/Blob.zig, src/bun.js/webcore/fetch.zig
Replaced conditional optional pattern matching with unconditional try expressions; now propagate allocation failures as exceptions
Package Manager Updates
src/install/PackageManager/UpdateRequest.zig
Replaced orelse-return patterns with try for string and array item conversions; removed defensive hasException() checks
Test Configuration
test/internal/ban-limits.json
Updated counters: "TODO: properly propagate exception upwards" reduced by 1, "globalThis.hasException" reduced by 2

Possibly related PRs

  • #23937 — Modifies the same string-slice APIs and ZigString/JSValue conversion signatures
  • #23889 — Changes toSlice/toSliceClone semantics and error propagation patterns
  • #23015 — Modifies src/bun.js/webcore/Blob.zig slice and allocator handling

Suggested reviewers

  • pfgithub
  • taylordotfish
  • nektro

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is largely incomplete. While it mentions removing a TODO, the 'How did you verify your code works?' section is empty and lacks verification details. Complete the 'How did you verify your code works?' section by describing testing approach, test results, or manual verification steps performed.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: updating JSValue.toSliceClone to use JSError instead of nullable types for error handling.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7a06dfc and ac1184d.

📒 Files selected for processing (8)
  • src/bun.js/api/JSBundler.zig (1 hunks)
  • src/bun.js/api/JSTranspiler.zig (1 hunks)
  • src/bun.js/api/glob.zig (1 hunks)
  • src/bun.js/bindings/JSValue.zig (1 hunks)
  • src/bun.js/webcore/Blob.zig (3 hunks)
  • src/bun.js/webcore/fetch.zig (1 hunks)
  • src/install/PackageManager/UpdateRequest.zig (1 hunks)
  • test/internal/ban-limits.json (1 hunks)
🧰 Additional context used
🧠 Learnings (23)
📓 Common learnings
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun's Zig codebase, `.js_undefined` is a valid way to represent JavaScript's undefined value when working with JSPromise.resolve() and similar JavaScript interop functions. This is the correct pattern to use rather than `jsc.JSValue.jsUndefined()`.
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Learnt from: nektro
Repo: oven-sh/bun PR: 22781
File: src/bun.js/bindings/CatchScope.zig:80-82
Timestamp: 2025-09-26T01:08:40.116Z
Learning: In Zig, when a function uses a bare error union return type like `!void`, the compiler infers the minimal error set from the function body. If the function only returns specific errors like `error.JSError`, the inferred error set will be `error{JSError}`, not `anyerror`. This makes `!void` functionally equivalent to explicitly specifying the error set when the function body only returns specific error values.
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 0
File: :0-0
Timestamp: 2025-09-02T19:17:26.376Z
Learning: In Bun's Zig codebase, when handling error unions where the same cleanup operation (like `rawFree`) needs to be performed regardless of success or failure, prefer using boolean folding with `else |err| switch (err)` over duplicating the cleanup call in multiple switch branches. This approach avoids code duplication while maintaining compile-time error checking.
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/allocators/allocation_scope.zig:79-85
Timestamp: 2025-09-02T18:09:21.647Z
Learning: In Bun's Zig codebase, prefer simple, direct error handling patterns. When an error type has only one variant (like FreeError with only NotAllocated), use a switch statement rather than generic catch blocks to leverage compile-time knowledge and avoid unnecessary complexity. Avoid duplicating function calls across error paths when a cleaner structure is possible.
📚 Learning: 2025-11-03T20:40:59.655Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.

Applied to files:

  • src/bun.js/webcore/Blob.zig
  • src/bun.js/api/JSBundler.zig
  • src/bun.js/webcore/fetch.zig
  • src/bun.js/api/glob.zig
  • src/install/PackageManager/UpdateRequest.zig
  • src/bun.js/bindings/JSValue.zig
  • src/bun.js/api/JSTranspiler.zig
📚 Learning: 2025-09-02T19:17:26.376Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 0
File: :0-0
Timestamp: 2025-09-02T19:17:26.376Z
Learning: In Bun's Zig codebase, when handling error unions where the same cleanup operation (like `rawFree`) needs to be performed regardless of success or failure, prefer using boolean folding with `else |err| switch (err)` over duplicating the cleanup call in multiple switch branches. This approach avoids code duplication while maintaining compile-time error checking.

Applied to files:

  • src/bun.js/webcore/Blob.zig
  • src/bun.js/api/JSBundler.zig
  • src/bun.js/webcore/fetch.zig
  • src/bun.js/api/glob.zig
  • src/bun.js/api/JSTranspiler.zig
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.

Applied to files:

  • src/bun.js/webcore/Blob.zig
  • src/bun.js/bindings/JSValue.zig
📚 Learning: 2025-11-03T20:43:06.996Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.

Applied to files:

  • src/bun.js/webcore/Blob.zig
  • src/bun.js/api/JSBundler.zig
  • src/bun.js/bindings/JSValue.zig
  • src/bun.js/api/JSTranspiler.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.

Applied to files:

  • src/bun.js/webcore/Blob.zig
  • src/bun.js/api/JSBundler.zig
  • src/bun.js/webcore/fetch.zig
  • src/bun.js/bindings/JSValue.zig
📚 Learning: 2025-09-25T22:07:13.851Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

Applied to files:

  • src/bun.js/webcore/Blob.zig
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().

Applied to files:

  • src/bun.js/webcore/Blob.zig
  • src/bun.js/api/JSBundler.zig
  • src/bun.js/bindings/JSValue.zig
  • src/bun.js/api/JSTranspiler.zig
📚 Learning: 2025-09-30T22:53:19.887Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 23117
File: src/bun.js/test/snapshot.zig:265-276
Timestamp: 2025-09-30T22:53:19.887Z
Learning: In Bun's snapshot testing (src/bun.js/test/snapshot.zig), multiple inline snapshots at the same line and column (same call position) must have identical values. However, multiple inline snapshots on the same line at different columns are allowed to have different values. The check is position-specific (line+col), not line-wide.

Applied to files:

  • src/bun.js/webcore/Blob.zig
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.

Applied to files:

  • src/bun.js/webcore/Blob.zig
  • src/bun.js/bindings/JSValue.zig
  • src/bun.js/api/JSTranspiler.zig
📚 Learning: 2025-09-02T18:09:21.647Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/allocators/allocation_scope.zig:79-85
Timestamp: 2025-09-02T18:09:21.647Z
Learning: In Bun's Zig codebase, prefer simple, direct error handling patterns. When an error type has only one variant (like FreeError with only NotAllocated), use a switch statement rather than generic catch blocks to leverage compile-time knowledge and avoid unnecessary complexity. Avoid duplicating function calls across error paths when a cleaner structure is possible.

Applied to files:

  • src/bun.js/api/JSBundler.zig
  • src/bun.js/webcore/fetch.zig
  • src/bun.js/api/JSTranspiler.zig
📚 Learning: 2025-09-12T18:16:50.754Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.

Applied to files:

  • src/bun.js/api/JSBundler.zig
  • src/bun.js/webcore/fetch.zig
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun's Zig codebase, `.js_undefined` is a valid way to represent JavaScript's undefined value when working with JSPromise.resolve() and similar JavaScript interop functions. This is the correct pattern to use rather than `jsc.JSValue.jsUndefined()`.

Applied to files:

  • src/bun.js/api/JSBundler.zig
  • src/bun.js/bindings/JSValue.zig
  • src/bun.js/api/JSTranspiler.zig
📚 Learning: 2025-09-06T03:37:41.154Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.

Applied to files:

  • src/bun.js/api/JSBundler.zig
📚 Learning: 2025-10-17T20:50:58.644Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.

Applied to files:

  • src/bun.js/api/JSBundler.zig
  • src/bun.js/webcore/fetch.zig
📚 Learning: 2025-09-19T19:55:22.427Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: src/valkey/js_valkey_functions.zig:853-854
Timestamp: 2025-09-19T19:55:22.427Z
Learning: In Bun's JSPromise API, `resolvedPromiseValue(globalThis: *JSGlobalObject, value: JSValue) JSValue` returns a JSValue directly (not a JSPromise that needs `.toJS()` called on it). This is different from `resolvedPromise()` which returns a *JSPromise that would need `.toJS()`.

Applied to files:

  • src/bun.js/api/JSBundler.zig
📚 Learning: 2025-10-26T04:50:17.892Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24086
File: src/bun.js/api/server.zig:2534-2535
Timestamp: 2025-10-26T04:50:17.892Z
Learning: In src/bun.js/api/server.zig, u32 is acceptable for route indices and websocket_context_index fields, as the practical limit of 4 billion contexts will never be reached.

Applied to files:

  • src/bun.js/webcore/fetch.zig
📚 Learning: 2025-11-12T05:43:49.804Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 24575
File: src/bun.js/api/bun/socket/Listener.zig:526-537
Timestamp: 2025-11-12T05:43:49.804Z
Learning: In Bun's socket Listener implementation (src/bun.js/api/bun/socket/Listener.zig), the getFD() method intentionally returns -1 (not js_undefined) for non-socket listener types such as named pipes, as this is the desired behavior for the fd property.

Applied to files:

  • src/bun.js/webcore/fetch.zig
📚 Learning: 2025-09-02T18:27:23.279Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/collections/multi_array_list.zig:24-24
Timestamp: 2025-09-02T18:27:23.279Z
Learning: The `#allocator` syntax in bun's custom Zig implementation is valid and does not require quoting with @"#allocator". Private fields using the `#` prefix work correctly throughout the codebase without special quoting syntax.

Applied to files:

  • src/bun.js/bindings/JSValue.zig
📚 Learning: 2025-10-11T15:19:30.301Z
Learnt from: mastermakrela
Repo: oven-sh/bun PR: 19167
File: src/bun.js/api.zig:31-31
Timestamp: 2025-10-11T15:19:30.301Z
Learning: The learnings about `pub const js = JSC.Codegen.JS<ClassName>` and re-exporting toJS/fromJS/fromJSDirect apply to class-based Zig bindings with constructors and prototype methods (e.g., those with `new` constructors). They do NOT apply to simple namespace-style API objects like TOMLObject, YAMLObject, and CSVObject which expose static functions via a `create()` method that returns a plain JS object.

Applied to files:

  • src/bun.js/bindings/JSValue.zig
  • src/bun.js/api/JSTranspiler.zig
📚 Learning: 2025-09-05T18:45:29.200Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey.zig:0-0
Timestamp: 2025-09-05T18:45:29.200Z
Learning: In JSValkeyClient.cloneWithoutConnecting() in src/valkey/js_valkey.zig, the address/username/password fields must be repointed to the duplicated connection_strings buffer to avoid use-after-free when the original client is destroyed. The original client properly frees connection_strings in ValkeyClient.deinit().

Applied to files:

  • src/bun.js/bindings/JSValue.zig
📚 Learning: 2025-11-11T22:55:04.070Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24571
File: src/css/selectors/parser.zig:908-916
Timestamp: 2025-11-11T22:55:04.070Z
Learning: In oven-sh/bun, CSS serialization uses an arena allocator. In src/css/selectors/parser.zig, functions like PseudoClass.toCss and PseudoElement.toCss intentionally do not call deinit on std.Io.Writer.Allocating, scratch buffers, or css.Printer because dest.allocator is an arena and these temporaries are reclaimed when the CSS pass completes. Only debug-only paths (e.g., DeclarationBlock.DebugFmt in src/css/declaration.zig) may explicitly deinit.

Applied to files:

  • src/bun.js/bindings/JSValue.zig
📚 Learning: 2025-09-26T01:10:04.233Z
Learnt from: nektro
Repo: oven-sh/bun PR: 22781
File: test/internal/ban-limits.json:25-26
Timestamp: 2025-09-26T01:10:04.233Z
Learning: The test/internal/ban-limits.json file contains accurate counts that are validated by tests - if the counts were incorrect, the associated tests would fail, so the values in this file reflect the actual current state of the codebase.

Applied to files:

  • test/internal/ban-limits.json
🔇 Additional comments (7)
src/bun.js/api/JSBundler.zig (1)

949-951: Thanks for wiring up the new JSError path.

Using catch with the existing panic keeps the previous invariant for non-string plugin returns while complying with the updated signature—looks good.

src/bun.js/webcore/Blob.zig (1)

3862-3865: Nice propagation via try.

Forwarding the new JSError union with try ensures JS exceptions surface correctly, and the joiner still owns the cloned slices via the allocator handle. 👍

Also applies to: 3957-3960

src/bun.js/webcore/fetch.zig (1)

482-506: Unix socket unix option now correctly uses throwing toSliceCloneWithAllocator

Using try socket_path.toSliceCloneWithAllocator(globalThis, allocator) cleanly propagates bun.JSError instead of relying on nullable return, and keeps ownership with unix_socket_path consistent with the existing deferred cleanup and FetchTasklet handoff.

test/internal/ban-limits.json (1)

20-27: Ban-limit counters updated consistently with refactor

The decreased counts for "TODO: properly propagate exception upwards" and "globalThis.hasException" align with the new throwing-path changes; since tests assert these values, this adjustment looks correct.

src/bun.js/api/glob.zig (1)

274-283: Glob constructor now uses throwing toSliceClone appropriately

Switching to const pat_str = @constCast((try pat_arg.toSliceClone(globalThis)).slice()); correctly adopts the new throwing API, and the buffer is still freed in finalize, so ownership and error propagation both look sound.

src/bun.js/api/JSTranspiler.zig (1)

369-405: Throwing toSliceCloneWithAllocator in exports.replace is wired correctly

Using try replacementKey.toSliceCloneWithAllocator(globalThis, allocator) plus errdefer slice.deinit() cleanly propagates JSErrors while retaining the cloned name bytes for the arena-backed replace_exports map on success.

src/install/PackageManager/UpdateRequest.zig (1)

58-71: UpdateRequest.fromJS now correctly propagates JSErrors from argument stringification

Swapping to try input.toSliceCloneWithAllocator(globalThis, allocator) / try item.toSliceCloneWithAllocator(...) ensures any toString/allocation failures surface as bun.JSError, while using the arena-backed allocator keeps temporary slices bounded to fromJS’s lifetime without leaks.


Comment @coderabbitai help to get the list of available commands and usage tips.

@Jarred-Sumner Jarred-Sumner merged commit 25b91e5 into main Nov 23, 2025
59 of 60 checks passed
@Jarred-Sumner Jarred-Sumner deleted the dylan/to-slice-clone-js-error branch November 23, 2025 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants