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

Jump to bottom

Cleanup how build errors are displayed slightly #7223

Merged
merged 11 commits into from
Nov 20, 2023
Merged

Cleanup how build errors are displayed slightly #7223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9e3c252
Cleanup error formatting a little
Jarred-Sumner Nov 19, 2023
18b6dd5
Add error for using import statement with CommonJS-only features
Jarred-Sumner Nov 19, 2023
da938ee
Update js_ast.zig
Jarred-Sumner Nov 19, 2023
f0058a1
Further tweaks to formatting, also print error.cause
Jarred-Sumner Nov 20, 2023
050f55f
Add some snapshot tests for errors
Jarred-Sumner Nov 20, 2023
95ac5b4
Make these snapshot tests
Jarred-Sumner Nov 20, 2023
2e5d0f4
Ignore mimalloc warnings
Jarred-Sumner Nov 20, 2023
1564767
Update error message parsing in bundling tests
Jarred-Sumner Nov 20, 2023
905c70a
Increase timeout on the test
Jarred-Sumner Nov 20, 2023
63cd560
Update expectBundled.ts
Jarred-Sumner Nov 20, 2023
e3c9b48
Update test
Jarred-Sumner Nov 20, 2023
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
69 changes: 50 additions & 19 deletions src/bun.js/javascript.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,12 @@ pub const SavedSourceMap = struct {
Output.errorWriter(),
logger.Kind.warn,
true,
false,
);
} else {
try fail.toData(path).writeFormat(
Output.errorWriter(),
logger.Kind.warn,
false,

false,
);
}
Expand Down Expand Up @@ -2022,6 +2021,10 @@ pub const VirtualMachine = struct {
if (!result.isEmptyOrUndefinedOrNull())
this.last_reported_error_for_dedupe = result;

var prev_had_errors = this.had_errors;
this.had_errors = false;
defer this.had_errors = prev_had_errors;

if (result.isException(this.global.vm())) {
var exception = @as(*Exception, @ptrCast(result.asVoid()));

Expand Down Expand Up @@ -2429,9 +2432,12 @@ pub const VirtualMachine = struct {
if (value.as(JSC.BuildMessage)) |build_error| {
defer Output.flush();
if (!build_error.logged) {
if (this.had_errors) {
writer.writeAll("\n") catch {};
}
build_error.msg.writeFormat(writer, allow_ansi_color) catch {};
writer.writeAll("\n") catch {};
build_error.logged = true;
writer.writeAll("\n") catch {};
}
this.had_errors = this.had_errors or build_error.msg.kind == .err;
if (exception_list != null) {
Expand All @@ -2443,8 +2449,12 @@ pub const VirtualMachine = struct {
} else if (value.as(JSC.ResolveMessage)) |resolve_error| {
defer Output.flush();
if (!resolve_error.logged) {
if (this.had_errors) {
writer.writeAll("\n") catch {};
}
resolve_error.msg.writeFormat(writer, allow_ansi_color) catch {};
resolve_error.logged = true;
writer.writeAll("\n") catch {};
}

this.had_errors = this.had_errors or resolve_error.msg.kind == .err;
Expand Down Expand Up @@ -2683,12 +2693,14 @@ pub const VirtualMachine = struct {
}
}

pub fn printErrorInstance(this: *VirtualMachine, error_instance: JSValue, exception_list: ?*ExceptionList, comptime Writer: type, writer: Writer, comptime allow_ansi_color: bool, comptime allow_side_effects: bool) !void {
pub fn printErrorInstance(this: *VirtualMachine, error_instance: JSValue, exception_list: ?*ExceptionList, comptime Writer: type, writer: Writer, comptime allow_ansi_color: bool, comptime allow_side_effects: bool) anyerror!void {
var exception_holder = ZigException.Holder.init();
var exception = exception_holder.zigException();
defer exception_holder.deinit();
this.remapZigException(exception, error_instance, exception_list);
var prev_had_errors = this.had_errors;
this.had_errors = true;
defer this.had_errors = prev_had_errors;

if (allow_side_effects) {
defer if (this.on_exception) |cb| {
Expand Down Expand Up @@ -2807,27 +2819,41 @@ pub const VirtualMachine = struct {
"info",
"pkg",
"errors",
"cause",
};

// This is usually unsafe to do, but we are protecting them each time first
var errors_to_append = std.ArrayList(JSC.JSValue).init(this.allocator);
defer {
for (errors_to_append.items) |err| {
err.unprotect();
}
errors_to_append.deinit();
}

if (error_instance != .zero and error_instance.isCell() and error_instance.jsType().canGet()) {
inline for (extra_fields) |field| {
if (error_instance.get(this.global, field)) |value| {
if (!value.isEmptyOrUndefinedOrNull()) {
const kind = value.jsType();
if (kind.isStringLike()) {
if (value.toStringOrNull(this.global)) |str| {
var zig_str = str.toSlice(this.global, bun.default_allocator);
defer zig_str.deinit();
try writer.print(comptime Output.prettyFmt(" {s}<d>: <r>\"{s}\"<r>\n", allow_ansi_color), .{ field, zig_str.slice() });
add_extra_line = true;
}
} else if (kind.isObject() or kind.isArray()) {
var bun_str = bun.String.empty;
defer bun_str.deref();
value.jsonStringify(this.global, 2, &bun_str); //2
try writer.print(comptime Output.prettyFmt(" {s}<d>: <r>{any}<r>\n", allow_ansi_color), .{ field, bun_str });
if (error_instance.getTruthy(this.global, field)) |value| {
const kind = value.jsType();
if (kind.isStringLike()) {
if (value.toStringOrNull(this.global)) |str| {
var zig_str = str.toSlice(this.global, bun.default_allocator);
defer zig_str.deinit();
try writer.print(comptime Output.prettyFmt(" {s}<d>: <r>\"{s}\"<r>\n", allow_ansi_color), .{ field, zig_str.slice() });
add_extra_line = true;
}
} else if (kind == .ErrorInstance and
// avoid infinite recursion
!prev_had_errors)
{
value.protect();
try errors_to_append.append(value);
} else if (kind.isObject() or kind.isArray()) {
var bun_str = bun.String.empty;
defer bun_str.deref();
value.jsonStringify(this.global, 2, &bun_str); //2
try writer.print(comptime Output.prettyFmt(" {s}<d>: <r>{any}<r>\n", allow_ansi_color), .{ field, bun_str });
add_extra_line = true;
}
}
}
Expand Down Expand Up @@ -2878,6 +2904,11 @@ pub const VirtualMachine = struct {
if (add_extra_line) try writer.writeAll("\n");

try printStackTrace(@TypeOf(writer), writer, exception.stack, allow_ansi_color);

for (errors_to_append.items) |err| {
try writer.writeAll("\n");
try this.printErrorInstance(err, exception_list, Writer, writer, allow_ansi_color, allow_side_effects);
}
}

fn printErrorNameAndMessage(_: *VirtualMachine, name: String, message: String, comptime Writer: type, writer: Writer, comptime allow_ansi_color: bool) !void {
Expand Down
2 changes: 1 addition & 1 deletion src/js_ast.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7265,7 +7265,7 @@ pub const Macro = struct {
this.source,
this.caller.loc,
this.allocator,
"cannot coerce {s} to Bun's AST. Please return a valid macro using the JSX syntax",
"cannot coerce {s} to Bun's AST. Please return a simpler type",
.{@tagName(value.jsType())},
) catch unreachable;
return error.MacroFailed;
Expand Down
4 changes: 3 additions & 1 deletion src/js_lexer.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3126,7 +3126,6 @@ pub fn rangeOfIdentifier(source: *const Source, loc: logger.Loc) logger.Range {
}

if (isIdentifierStart(cursor.c) or cursor.c == '\\') {
defer r.len = @as(i32, @intCast(cursor.i));
while (iter.next(&cursor)) {
if (cursor.c == '\\') {

Expand All @@ -3144,9 +3143,12 @@ pub fn rangeOfIdentifier(source: *const Source, loc: logger.Loc) logger.Range {
}
}
} else if (!isIdentifierContinue(cursor.c)) {
r.len = @as(i32, @intCast(cursor.i));
return r;
}
}

r.len = @as(i32, @intCast(cursor.i));
}

// const offset = @intCast(usize, loc.start);
Expand Down
46 changes: 46 additions & 0 deletions src/js_parser.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3635,6 +3635,52 @@ pub const Parser = struct {
wrapper_expr = .{
.bun_js = {},
};

const import_record: ?*const ImportRecord = brk: {
for (p.import_records.items) |*import_record| {
if (import_record.is_internal or import_record.is_unused) continue;
if (import_record.kind == .stmt) break :brk import_record;
}

break :brk null;
};

// make it an error to use an import statement with a commonjs exports usage
if (import_record) |record| {
// find the usage of the export symbol

var notes = ListManaged(logger.Data).init(p.allocator);

try notes.append(logger.Data{
.text = try std.fmt.allocPrint(p.allocator, "Try require({}) instead", .{strings.QuotedFormatter{ .text = record.path.text }}),
});

if (uses_module_ref) {
try notes.append(logger.Data{
.text = "This file is CommonJS because 'module' was used",
});
}

if (uses_exports_ref) {
try notes.append(logger.Data{
.text = "This file is CommonJS because 'exports' was used",
});
}

if (p.has_top_level_return) {
try notes.append(logger.Data{
.text = "This file is CommonJS because top-level return was used",
});
}

if (p.has_with_scope) {
try notes.append(logger.Data{
.text = "This file is CommonJS because a \"with\" statement is used",
});
}

try p.log.addRangeErrorWithNotes(p.source, record.range, "Cannot use import statement with CommonJS-only features", notes.items);
}
} else if (!p.options.bundle and !p.options.features.commonjs_at_runtime and (!p.options.transform_only or p.options.features.dynamic_require)) {
if (p.options.legacy_transform_require_to_import or p.options.features.dynamic_require) {
var args = p.allocator.alloc(Expr, 2) catch unreachable;
Expand Down
Loading
Loading