From 85a2ebb71762da9fa3fdf7335ea76cb9c1fbfc77 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Sat, 11 Oct 2025 08:23:25 -0700 Subject: [PATCH] fix #23470 (#23471) ### What does this PR do? `CompileResult` error message memory was not managed correctly. Fixes #23470 ### How did you verify your code works? Manually --- src/StandaloneModuleGraph.zig | 93 +++++++++++++++++++++++------------ src/bundler/bundle_v2.zig | 10 ++-- src/cli/build_command.zig | 2 +- test/cli/init/init.test.ts | 6 +-- 4 files changed, 71 insertions(+), 40 deletions(-) diff --git a/src/StandaloneModuleGraph.zig b/src/StandaloneModuleGraph.zig index b09e527d0c..72b7b38cff 100644 --- a/src/StandaloneModuleGraph.zig +++ b/src/StandaloneModuleGraph.zig @@ -525,15 +525,48 @@ pub const StandaloneModuleGraph = struct { pub const CompileResult = union(enum) { success: void, - error_message: []const u8, - pub fn fail(msg: []const u8) CompileResult { - return .{ .error_message = msg }; + err: Error, + + const Error = union(enum) { + message: []const u8, + reason: Reason, + + pub const Reason = enum { + no_entry_point, + no_output_files, + + pub fn message(this: Reason) []const u8 { + return switch (this) { + .no_entry_point => "No entry point found for compilation", + .no_output_files => "No output files to bundle", + }; + } + }; + + pub fn slice(this: *const Error) []const u8 { + return switch (this.*) { + .message => this.message, + .reason => this.reason.message(), + }; + } + }; + + pub fn fail(reason: Error.Reason) CompileResult { + return .{ .err = .{ .reason = reason } }; + } + + pub fn failFmt(comptime fmt: []const u8, args: anytype) CompileResult { + return .{ .err = .{ .message = bun.handleOom(std.fmt.allocPrint(bun.default_allocator, fmt, args)) } }; } pub fn deinit(this: *const @This()) void { - if (this.* == .error_message) { - bun.default_allocator.free(this.error_message); + switch (this.*) { + .success => {}, + .err => switch (this.err) { + .message => bun.default_allocator.free(this.err.message), + .reason => {}, + }, } } }; @@ -951,9 +984,9 @@ pub const StandaloneModuleGraph = struct { self_exe_path: ?[]const u8, ) !CompileResult { const bytes = toBytes(allocator, module_prefix, output_files, output_format, compile_exec_argv) catch |err| { - return CompileResult.fail(std.fmt.allocPrint(allocator, "failed to generate module graph bytes: {s}", .{@errorName(err)}) catch "failed to generate module graph bytes"); + return CompileResult.failFmt("failed to generate module graph bytes: {s}", .{@errorName(err)}); }; - if (bytes.len == 0) return CompileResult.fail("no output files to bundle"); + if (bytes.len == 0) return CompileResult.fail(.no_output_files); defer allocator.free(bytes); var free_self_exe = false; @@ -962,28 +995,26 @@ pub const StandaloneModuleGraph = struct { break :brk bun.handleOom(allocator.dupeZ(u8, path)); } else if (target.isDefault()) bun.selfExePath() catch |err| { - return CompileResult.fail(std.fmt.allocPrint(allocator, "failed to get self executable path: {s}", .{@errorName(err)}) catch "failed to get self executable path"); + return CompileResult.failFmt("failed to get self executable path: {s}", .{@errorName(err)}); } else blk: { var exe_path_buf: bun.PathBuffer = undefined; - var version_str_buf: [1024]u8 = undefined; - const version_str = std.fmt.bufPrintZ(&version_str_buf, "{}", .{target}) catch { - return CompileResult.fail("failed to format target version string"); - }; + const version_str = bun.handleOom(std.fmt.allocPrintZ(allocator, "{}", .{target})); + defer allocator.free(version_str); + var needs_download: bool = true; const dest_z = target.exePath(&exe_path_buf, version_str, env, &needs_download); if (needs_download) { target.downloadToPath(env, allocator, dest_z) catch |err| { - const msg = switch (err) { - error.TargetNotFound => std.fmt.allocPrint(allocator, "Target platform '{}' is not available for download. Check if this version of Bun supports this target.", .{target}) catch "Target platform not available for download", - error.NetworkError => std.fmt.allocPrint(allocator, "Network error downloading executable for '{}'. Check your internet connection and proxy settings.", .{target}) catch "Network error downloading executable", - error.InvalidResponse => std.fmt.allocPrint(allocator, "Downloaded file for '{}' appears to be corrupted. Please try again.", .{target}) catch "Downloaded file is corrupted", - error.ExtractionFailed => std.fmt.allocPrint(allocator, "Failed to extract executable for '{}'. The download may be incomplete.", .{target}) catch "Failed to extract downloaded executable", - error.UnsupportedTarget => std.fmt.allocPrint(allocator, "Target '{}' is not supported", .{target}) catch "Unsupported target", - else => std.fmt.allocPrint(allocator, "Failed to download '{}': {s}", .{ target, @errorName(err) }) catch "Download failed", + return switch (err) { + error.TargetNotFound => CompileResult.failFmt("Target platform '{}' is not available for download. Check if this version of Bun supports this target.", .{target}), + error.NetworkError => CompileResult.failFmt("Network error downloading executable for '{}'. Check your internet connection and proxy settings.", .{target}), + error.InvalidResponse => CompileResult.failFmt("Downloaded file for '{}' appears to be corrupted. Please try again.", .{target}), + error.ExtractionFailed => CompileResult.failFmt("Failed to extract executable for '{}'. The download may be incomplete.", .{target}), + error.UnsupportedTarget => CompileResult.failFmt("Target '{}' is not supported", .{target}), + else => CompileResult.failFmt("Failed to download '{}': {s}", .{ target, @errorName(err) }), }; - return CompileResult.fail(msg); }; } @@ -1013,7 +1044,7 @@ pub const StandaloneModuleGraph = struct { // Get the current path of the temp file var temp_buf: bun.PathBuffer = undefined; const temp_path = bun.getFdPath(fd, &temp_buf) catch |err| { - return CompileResult.fail(std.fmt.allocPrint(allocator, "Failed to get temp file path: {s}", .{@errorName(err)}) catch "Failed to get temp file path"); + return CompileResult.failFmt("Failed to get temp file path: {s}", .{@errorName(err)}); }; // Build the absolute destination path @@ -1021,7 +1052,7 @@ pub const StandaloneModuleGraph = struct { // Get the current working directory and join with outfile var cwd_buf: bun.PathBuffer = undefined; const cwd_path = bun.getcwd(&cwd_buf) catch |err| { - return CompileResult.fail(std.fmt.allocPrint(allocator, "Failed to get current directory: {s}", .{@errorName(err)}) catch "Failed to get current directory"); + return CompileResult.failFmt("Failed to get current directory: {s}", .{@errorName(err)}); }; const dest_path = if (std.fs.path.isAbsolute(outfile)) outfile @@ -1049,12 +1080,12 @@ pub const StandaloneModuleGraph = struct { const err = bun.windows.Win32Error.get(); if (err.toSystemErrno()) |sys_err| { if (sys_err == .EISDIR) { - return CompileResult.fail(std.fmt.allocPrint(allocator, "{s} is a directory. Please choose a different --outfile or delete the directory", .{outfile}) catch "outfile is a directory"); + return CompileResult.failFmt("{s} is a directory. Please choose a different --outfile or delete the directory", .{outfile}); } else { - return CompileResult.fail(std.fmt.allocPrint(allocator, "failed to move executable to {s}: {s}", .{ dest_path, @tagName(sys_err) }) catch "failed to move executable"); + return CompileResult.failFmt("failed to move executable to {s}: {s}", .{ dest_path, @tagName(sys_err) }); } } else { - return CompileResult.fail(std.fmt.allocPrint(allocator, "failed to move executable to {s}", .{dest_path}) catch "failed to move executable"); + return CompileResult.failFmt("failed to move executable to {s}", .{dest_path}); } } @@ -1076,7 +1107,7 @@ pub const StandaloneModuleGraph = struct { windows_options.description, windows_options.copyright, ) catch |err| { - return CompileResult.fail(std.fmt.allocPrint(allocator, "Failed to set Windows metadata: {s}", .{@errorName(err)}) catch "Failed to set Windows metadata"); + return CompileResult.failFmt("Failed to set Windows metadata: {s}", .{@errorName(err)}); }; } return .success; @@ -1084,14 +1115,14 @@ pub const StandaloneModuleGraph = struct { var buf: bun.PathBuffer = undefined; const temp_location = bun.getFdPath(fd, &buf) catch |err| { - return CompileResult.fail(std.fmt.allocPrint(allocator, "failed to get path for fd: {s}", .{@errorName(err)}) catch "failed to get path for file descriptor"); + return CompileResult.failFmt("failed to get path for fd: {s}", .{@errorName(err)}); }; const temp_posix = std.posix.toPosixPath(temp_location) catch |err| { - return CompileResult.fail(std.fmt.allocPrint(allocator, "path too long: {s}", .{@errorName(err)}) catch "path too long"); + return CompileResult.failFmt("path too long: {s}", .{@errorName(err)}); }; const outfile_basename = std.fs.path.basename(outfile); const outfile_posix = std.posix.toPosixPath(outfile_basename) catch |err| { - return CompileResult.fail(std.fmt.allocPrint(allocator, "outfile name too long: {s}", .{@errorName(err)}) catch "outfile name too long"); + return CompileResult.failFmt("outfile name too long: {s}", .{@errorName(err)}); }; bun.sys.moveFileZWithHandle( @@ -1107,9 +1138,9 @@ pub const StandaloneModuleGraph = struct { _ = Syscall.unlink(&temp_posix); if (err == error.IsDir or err == error.EISDIR) { - return CompileResult.fail(std.fmt.allocPrint(allocator, "{s} is a directory. Please choose a different --outfile or delete the directory", .{outfile}) catch "outfile is a directory"); + return CompileResult.failFmt("{s} is a directory. Please choose a different --outfile or delete the directory", .{outfile}); } else { - return CompileResult.fail(std.fmt.allocPrint(allocator, "failed to rename {s} to {s}: {s}", .{ temp_location, outfile, @errorName(err) }) catch "failed to rename file"); + return CompileResult.failFmt("failed to rename {s} to {s}: {s}", .{ temp_location, outfile, @errorName(err) }); } }; diff --git a/src/bundler/bundle_v2.zig b/src/bundler/bundle_v2.zig index 0e7aa8f698..faaeb043af 100644 --- a/src/bundler/bundle_v2.zig +++ b/src/bundler/bundle_v2.zig @@ -1946,7 +1946,7 @@ pub const BundleV2 = struct { break :brk i; } } - return bun.StandaloneModuleGraph.CompileResult.fail("No entry point found for compilation"); + return bun.StandaloneModuleGraph.CompileResult.fail(.no_entry_point); }; const output_file = &output_files.items[entry_point_index]; @@ -1990,12 +1990,12 @@ pub const BundleV2 = struct { if (Environment.isPosix and !(dirname.len == 0 or strings.eqlComptime(dirname, "."))) { // On POSIX, makeOpenPath and change root_dir root_dir = root_dir.makeOpenPath(dirname, .{}) catch |err| { - return bun.StandaloneModuleGraph.CompileResult.fail(bun.handleOom(std.fmt.allocPrint(bun.default_allocator, "Failed to open output directory {s}: {s}", .{ dirname, @errorName(err) }))); + return bun.StandaloneModuleGraph.CompileResult.failFmt("Failed to open output directory {s}: {s}", .{ dirname, @errorName(err) }); }; } else if (Environment.isWindows and !(dirname.len == 0 or strings.eqlComptime(dirname, "."))) { // On Windows, ensure directories exist but don't change root_dir _ = bun.makePath(root_dir, dirname) catch |err| { - return bun.StandaloneModuleGraph.CompileResult.fail(bun.handleOom(std.fmt.allocPrint(bun.default_allocator, "Failed to create output directory {s}: {s}", .{ dirname, @errorName(err) }))); + return bun.StandaloneModuleGraph.CompileResult.failFmt("Failed to create output directory {s}: {s}", .{ dirname, @errorName(err) }); }; } @@ -2044,7 +2044,7 @@ pub const BundleV2 = struct { else null, ) catch |err| { - return bun.StandaloneModuleGraph.CompileResult.fail(bun.handleOom(std.fmt.allocPrint(bun.default_allocator, "{s}", .{@errorName(err)}))); + return bun.StandaloneModuleGraph.CompileResult.failFmt("{s}", .{@errorName(err)}); }; if (result == .success) { @@ -2139,7 +2139,7 @@ pub const BundleV2 = struct { defer compile_result.deinit(); if (compile_result != .success) { - bun.handleOom(this.log.addError(null, Logger.Loc.Empty, bun.handleOom(this.log.msgs.allocator.dupe(u8, compile_result.error_message)))); + bun.handleOom(this.log.addError(null, Logger.Loc.Empty, bun.handleOom(this.log.msgs.allocator.dupe(u8, compile_result.err.slice())))); this.result.value.deinit(); this.result = .{ .err = error.CompilationFailed }; } diff --git a/src/cli/build_command.zig b/src/cli/build_command.zig index 5590ef1581..4fde46640d 100644 --- a/src/cli/build_command.zig +++ b/src/cli/build_command.zig @@ -435,7 +435,7 @@ pub const BuildCommand = struct { }; if (result != .success) { - Output.printErrorln("{s}", .{result.error_message}); + Output.printErrorln("{s}", .{result.err.slice()}); Global.exit(1); } diff --git a/test/cli/init/init.test.ts b/test/cli/init/init.test.ts index 6d6b67faad..1cabbaf20f 100644 --- a/test/cli/init/init.test.ts +++ b/test/cli/init/init.test.ts @@ -243,7 +243,7 @@ import path from "path"; expect(pkg).toHaveProperty("devDependencies.@types/react-dom"); expect(fs.existsSync(path.join(temp, "src"))).toBe(true); - expect(fs.existsSync(path.join(temp, "src/index.tsx"))).toBe(true); + expect(fs.existsSync(path.join(temp, "src/index.ts"))).toBe(true); expect(fs.existsSync(path.join(temp, "tsconfig.json"))).toBe(true); }, 30_000); @@ -267,7 +267,7 @@ import path from "path"; expect(pkg).toHaveProperty("dependencies.bun-plugin-tailwind"); expect(fs.existsSync(path.join(temp, "src"))).toBe(true); - expect(fs.existsSync(path.join(temp, "src/index.tsx"))).toBe(true); + expect(fs.existsSync(path.join(temp, "src/index.ts"))).toBe(true); }, 30_000); test("bun init --react=shadcn works", async () => { @@ -291,7 +291,7 @@ import path from "path"; expect(pkg).toHaveProperty("dependencies.bun-plugin-tailwind"); expect(fs.existsSync(path.join(temp, "src"))).toBe(true); - expect(fs.existsSync(path.join(temp, "src/index.tsx"))).toBe(true); + expect(fs.existsSync(path.join(temp, "src/index.ts"))).toBe(true); expect(fs.existsSync(path.join(temp, "src/components"))).toBe(true); expect(fs.existsSync(path.join(temp, "src/components/ui"))).toBe(true); }, 30_000);