Skip to content
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

zig: make throwTODO use JSError #15264

Merged
merged 2 commits into from
Nov 20, 2024
Merged

zig: make throwTODO use JSError #15264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4df5572
zig: make throwTODO use JSError
nektro Nov 19, 2024
d276df5
Apply suggestions from code review
nektro Nov 20, 2024
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
6 changes: 2 additions & 4 deletions src/bun.js/api/BunObject.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2654,8 +2654,7 @@ pub const Crypto = struct {
BoringSSL.ERR_clear_error();
return globalThis.throwValue2(instance);
} else {
globalThis.throwTODO("HMAC is not supported for this algorithm yet");
return error.JSError;
return globalThis.throwTODO("HMAC is not supported for this algorithm yet");
}
}
return error.JSError;
Expand Down Expand Up @@ -3465,8 +3464,7 @@ pub fn mmapFile(
callframe: *JSC.CallFrame,
) bun.JSError!JSC.JSValue {
if (comptime Environment.isWindows) {
globalThis.throwTODO("mmapFile is not supported on Windows");
return JSC.JSValue.zero;
return globalThis.throwTODO("mmapFile is not supported on Windows");
}

const arguments_ = callframe.arguments(2);
Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/api/bun/socket.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2305,7 +2305,7 @@ fn NewSocket(comptime ssl: bool) type {
const length_value = args[2];

if (encoding_value != .undefined and (offset_value != .undefined or length_value != .undefined)) {
globalObject.throwTODO("Support encoding with offset and length altogether. Only either encoding or offset, length is supported, but not both combinations yet.");
globalObject.throwTODO("Support encoding with offset and length altogether. Only either encoding or offset, length is supported, but not both combinations yet.") catch {};
Copy link
Member

@dylan-conway dylan-conway Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we change writeOrEnd to have an error union return type instead of catch {};? Same with stdio.zig extract?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps, yeah it seems this whole function is using .fail for exceptions
@cirospaciari would likely know more on the prospect of that change

return .fail;
}

Expand Down
4 changes: 2 additions & 2 deletions src/bun.js/api/bun/spawn/stdio.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,12 +387,12 @@ pub const Stdio = union(enum) {

switch (req.ptr) {
.File, .Blob => {
globalThis.throwTODO("Support fd/blob backed ReadableStream in spawn stdin. See https://github.com/oven-sh/bun/issues/8049");
globalThis.throwTODO("Support fd/blob backed ReadableStream in spawn stdin. See https://github.com/oven-sh/bun/issues/8049") catch {};
return false;
},
.Direct, .JavaScript, .Bytes => {
// out_stdio.* = .{ .connect = req };
globalThis.throwTODO("Re-enable ReadableStream support in spawn stdin. ");
globalThis.throwTODO("Re-enable ReadableStream support in spawn stdin. ") catch {};
return false;
},
.Invalid => {
Expand Down
3 changes: 1 addition & 2 deletions src/bun.js/api/server.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,7 @@ const StaticRoute = struct {

.Blob, .InternalBlob, .WTFStringImpl => {
if (response.body.value == .Blob and response.body.value.Blob.needsToReadFile()) {
globalThis.throwTODO("TODO: support Bun.file(path) in static routes");
return error.JSError;
return globalThis.throwTODO("TODO: support Bun.file(path) in static routes");
}
var blob = response.body.value.use();
blob.globalThis = globalThis;
Expand Down
16 changes: 4 additions & 12 deletions src/bun.js/bindings/bindings.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2962,10 +2962,10 @@ pub const JSGlobalObject = opaque {
return .zero;
}

pub fn throwTODO(this: *JSGlobalObject, msg: []const u8) void {
pub fn throwTODO(this: *JSGlobalObject, msg: []const u8) bun.JSError {
const err = this.createErrorInstance("{s}", .{msg});
err.put(this, ZigString.static("name"), bun.String.static("TODOError").toJS(this));
this.throwValue(err);
return this.throwValue2(err);
}

pub const throwTerminationException = JSGlobalObject__throwTerminationException;
Expand All @@ -2980,21 +2980,13 @@ pub const JSGlobalObject = opaque {
}

/// Deprecated: use `throwInvalidArguments2`
pub fn throwInvalidArguments(
this: *JSGlobalObject,
comptime fmt: [:0]const u8,
args: anytype,
) void {
pub fn throwInvalidArguments(this: *JSGlobalObject, comptime fmt: [:0]const u8, args: anytype) void {
const err = JSC.toInvalidArguments(fmt, args, this);
this.vm().throwError(this, err);
}

/// New system for throwing errors: returning bun.JSError
pub fn throwInvalidArguments2(
this: *JSGlobalObject,
comptime fmt: [:0]const u8,
args: anytype,
) bun.JSError {
pub fn throwInvalidArguments2(this: *JSGlobalObject, comptime fmt: [:0]const u8, args: anytype) bun.JSError {
const err = JSC.toInvalidArguments(fmt, args, this);
return this.vm().throwError2(this, err);
}
Expand Down
6 changes: 3 additions & 3 deletions src/codegen/generate-node-errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ listHeader += `
`;

zig += `


extern fn Bun__createErrorWithCode(globalThis: *JSC.JSGlobalObject, code: Error, message: *bun.String) JSC.JSValue;

/// Creates an Error object with the given error code.
/// Derefs the message string.
pub fn toJS(this: Error, globalThis: *JSC.JSGlobalObject, message: *bun.String) JSC.JSValue {
Expand All @@ -110,7 +110,7 @@ zig += `
}

pub fn throw(this: Error, globalThis: *JSC.JSGlobalObject, comptime fmt_str: [:0]const u8, args: anytype) void {
globalThis.throwValue(fmt(this, globalThis, fmt_str, args));
globalThis.throwValue(fmt(this, globalThis, fmt_str, args));
}

};
Expand Down
3 changes: 1 addition & 2 deletions src/install/dependency.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,7 @@ pub const Version = struct {
}
},
else => {
globalThis.throwTODO("Unsupported dependency type");
return error.JSError;
return globalThis.throwTODO("Unsupported dependency type");
},
}

Expand Down
56 changes: 18 additions & 38 deletions src/shell/interpreter.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1185,44 +1185,26 @@ pub const Interpreter = struct {
}
};

pub fn createShellInterpreter(
globalThis: *JSC.JSGlobalObject,
callframe: *JSC.CallFrame,
) bun.JSError!JSValue {
pub fn createShellInterpreter(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSValue {
const allocator = bun.default_allocator;
const arguments_ = callframe.arguments(3);
var arguments = JSC.Node.ArgumentsSlice.init(globalThis.bunVM(), arguments_.slice());

const resolve = arguments.nextEat() orelse {
globalThis.throw("shell: expected 3 arguments, got 0", .{});
return .undefined;
};
const resolve = arguments.nextEat() orelse return globalThis.throw2("shell: expected 3 arguments, got 0", .{});

const reject = arguments.nextEat() orelse {
globalThis.throw("shell: expected 3 arguments, got 0", .{});
return .undefined;
};
const reject = arguments.nextEat() orelse return globalThis.throw2("shell: expected 3 arguments, got 0", .{});

const parsed_shell_script_js = arguments.nextEat() orelse {
globalThis.throw("shell: expected 3 arguments, got 0", .{});
return .undefined;
};
const parsed_shell_script_js = arguments.nextEat() orelse return globalThis.throw2("shell: expected 3 arguments, got 0", .{});

const parsed_shell_script = parsed_shell_script_js.as(ParsedShellScript) orelse {
globalThis.throw("shell: expected a ParsedShellScript", .{});
return .undefined;
};
const parsed_shell_script = parsed_shell_script_js.as(ParsedShellScript) orelse return globalThis.throw2("shell: expected a ParsedShellScript", .{});

var shargs: *ShellArgs = undefined;
var jsobjs: std.ArrayList(JSValue) = std.ArrayList(JSValue).init(allocator);
var quiet: bool = false;
var cwd: ?bun.String = null;
var export_env: ?EnvMap = null;

if (parsed_shell_script.args == null) {
globalThis.throw("shell: shell args is null, this is a bug in Bun. Please file a GitHub issue.", .{});
return .undefined;
}
if (parsed_shell_script.args == null) return globalThis.throw2("shell: shell args is null, this is a bug in Bun. Please file a GitHub issue.", .{});

parsed_shell_script.take(
globalThis,
Expand Down Expand Up @@ -1253,8 +1235,7 @@ pub const Interpreter = struct {
if (export_env) |*ee| ee.deinit();
if (cwd) |*cc| cc.deref();
shargs.deinit();
throwShellErr(e, .{ .js = globalThis.bunVM().event_loop });
return .zero;
return try throwShellErr(e, .{ .js = globalThis.bunVM().event_loop });
},
};

Expand All @@ -1264,7 +1245,7 @@ pub const Interpreter = struct {
if (cwd) |*cc| cc.deref();
shargs.deinit();
interpreter.finalize();
return .zero;
return error.JSError;
}

interpreter.flags.quiet = quiet;
Expand Down Expand Up @@ -1477,8 +1458,7 @@ pub const Interpreter = struct {
null,
)) {
.err => |*e| {
throwShellErr(e, .{ .mini = mini });
return 1;
e.throwMini();
},
.result => |i| i,
};
Expand Down Expand Up @@ -1547,8 +1527,7 @@ pub const Interpreter = struct {
null,
)) {
.err => |*e| {
throwShellErr(e, .{ .mini = mini });
return 1;
e.throwMini();
},
.result => |i| i,
};
Expand Down Expand Up @@ -1648,8 +1627,7 @@ pub const Interpreter = struct {
if (this.setupIOBeforeRun().asErr()) |e| {
defer this.deinitEverything();
const shellerr = bun.shell.ShellErr.newSys(e);
throwShellErr(&shellerr, .{ .js = globalThis.bunVM().event_loop });
return .undefined;
return try throwShellErr(&shellerr, .{ .js = globalThis.bunVM().event_loop });
}
incrPendingActivityFlag(&this.has_pending_activity);

Expand Down Expand Up @@ -2771,7 +2749,7 @@ pub const Interpreter = struct {
}

pub fn throw(this: *const State, err: *const bun.shell.ShellErr) void {
throwShellErr(err, this.eventLoop());
throwShellErr(err, this.eventLoop()) catch {}; //TODO:
}

pub fn rootIO(this: *const State) *const IO {
Expand Down Expand Up @@ -5750,7 +5728,7 @@ pub const Interpreter = struct {
}

pub inline fn throw(this: *const Builtin, err: *const bun.shell.ShellErr) void {
this.parentCmd().base.throw(err);
this.parentCmd().base.throw(err) catch {};
}

pub inline fn parentCmd(this: *const Builtin) *const Cmd {
Expand Down Expand Up @@ -12221,11 +12199,13 @@ inline fn fastMod(val: anytype, comptime rhs: comptime_int) @TypeOf(val) {
return val & (rhs - 1);
}

fn throwShellErr(e: *const bun.shell.ShellErr, event_loop: JSC.EventLoopHandle) void {
switch (event_loop) {
/// 'js' event loop will always return JSError
/// 'mini' event loop will always return noreturn and exit 1
fn throwShellErr(e: *const bun.shell.ShellErr, event_loop: JSC.EventLoopHandle) bun.JSError!noreturn {
return switch (event_loop) {
.mini => e.throwMini(),
.js => e.throwJS(event_loop.js.global),
}
};
}

pub const ReadChunkAction = enum {
Expand Down
12 changes: 6 additions & 6 deletions src/shell/shell.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,30 +83,30 @@ pub const ShellErr = union(enum) {
}
}

pub fn throwJS(this: *const @This(), globalThis: *JSC.JSGlobalObject) void {
pub fn throwJS(this: *const @This(), globalThis: *JSC.JSGlobalObject) bun.JSError {
defer this.deinit(bun.default_allocator);
switch (this.*) {
.sys => {
const err = this.sys.toErrorInstance(globalThis);
globalThis.throwValue(err);
return globalThis.throwValue2(err);
},
.custom => {
var str = JSC.ZigString.init(this.custom);
str.markUTF8();
const err_value = str.toErrorInstance(globalThis);
globalThis.throwValue(err_value);
return globalThis.throwValue2(err_value);
// this.bunVM().allocator.free(JSC.ZigString.untagged(str._unsafe_ptr_do_not_use)[0..str.len]);
},
.invalid_arguments => {
globalThis.throwInvalidArguments("{s}", .{this.invalid_arguments.val});
return globalThis.throwInvalidArguments2("{s}", .{this.invalid_arguments.val});
},
.todo => {
globalThis.throwTODO(this.todo);
return globalThis.throwTODO(this.todo);
},
}
}

pub fn throwMini(this: @This()) void {
pub fn throwMini(this: @This()) noreturn {
defer this.deinit(bun.default_allocator);
switch (this) {
.sys => |err| {
Expand Down