From 0f9c9e2089fae75864524bc68d6e5a4e5d92c311 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 12 Jan 2026 10:17:28 +0800 Subject: [PATCH] Improve crash handling This adds a crash handler which reports a crash (if telemetry is enabled). On a crash, this looks for `curl` (using the PATH env), and forks the process to then call execve. This relies on a new endpoint to be setup to accept the "report". Also, we include very little data..I figured just knowing about crashes would be a good place to start. A panic handler is provided, which override's Zig default handler and hooks into the crash handler. An `assert` function is added and hooks into the crash handler. This is currently only used in one place (Session.zig) to demonstrate its use. In addition to reporting a failed assert, the assert aborts execution in ReleaseFast (as opposed to an undefined behavior with std.debug.assert). I want to hook this into the v8 global error handler, but only after direct_v8 is merged. Much of this is inspired by bun's code. They have their own assert (1) and a [more sophisticated] crashHandler (2). : (1) https://github.com/oven-sh/bun/blob/beccd01647ec4fcb5895be5c5fb88f5254dc1e8d/src/bun.zig#L2987 (2) https://github.com/oven-sh/bun/blob/beccd01647ec4fcb5895be5c5fb88f5254dc1e8d/src/crash_handler.zig#L198 --- build.zig | 1 + src/browser/Session.zig | 6 +- src/crash_handler.zig | 129 ++++++++++++++++++++++++++++++++++++ src/lightpanda.zig | 20 ++++++ src/main.zig | 1 + src/telemetry/telemetry.zig | 6 +- 6 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 src/crash_handler.zig diff --git a/build.zig b/build.zig index 18e33961..7d97728a 100644 --- a/build.zig +++ b/build.zig @@ -48,6 +48,7 @@ pub fn build(b: *Build) !void { .sanitize_c = enable_csan, .sanitize_thread = enable_tsan, }); + mod.addImport("lightpanda", mod); // allow circular "lightpanda" import try addDependencies(b, mod, opts, prebuilt_v8_path); diff --git a/src/browser/Session.zig b/src/browser/Session.zig index ca0002f4..340d6b61 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -17,6 +17,7 @@ // along with this program. If not, see . const std = @import("std"); +const lp = @import("lightpanda"); const log = @import("../log.zig"); @@ -92,7 +93,7 @@ pub fn deinit(self: *Session) void { // NOTE: the caller is not the owner of the returned value, // the pointer on Page is just returned as a convenience pub fn createPage(self: *Session) !*Page { - std.debug.assert(self.page == null); + lp.assert(self.page == null, "Session.createPage - page not null", .{}); const page_arena = &self.browser.page_arena; _ = page_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 }); @@ -116,8 +117,7 @@ pub fn createPage(self: *Session) !*Page { pub fn removePage(self: *Session) void { // Inform CDP the page is going to be removed, allowing other worlds to remove themselves before the main one self.browser.notification.dispatch(.page_remove, .{}); - - std.debug.assert(self.page != null); + lp.assert(self.page != null, "Session.removePage - page is null", .{}); self.page.?.deinit(); self.page = null; diff --git a/src/crash_handler.zig b/src/crash_handler.zig new file mode 100644 index 00000000..a8e22130 --- /dev/null +++ b/src/crash_handler.zig @@ -0,0 +1,129 @@ +const std = @import("std"); +const lp = @import("lightpanda"); +const builtin = @import("builtin"); + +const abort = std.posix.abort; + +// tracks how deep within a panic we're panicling +var panic_level: usize = 0; + +// Locked to avoid interleaving panic messages from multiple threads. +var panic_mutex = std.Thread.Mutex{}; + +// overwrite's Zig default panic handler +pub fn panic(msg: []const u8, _: ?*std.builtin.StackTrace, begin_addr: ?usize) noreturn { + @branchHint(.cold); + crash(msg, .{ .source = "global" }, begin_addr orelse @returnAddress()); +} + +pub noinline fn crash( + reason: []const u8, + args: anytype, + begin_addr: usize, +) noreturn { + @branchHint(.cold); + + nosuspend switch (panic_level) { + 0 => { + panic_level = panic_level + 1; + + { + panic_mutex.lock(); + defer panic_mutex.unlock(); + + var writer_w = std.fs.File.stderr().writerStreaming(&.{}); + const writer = &writer_w.interface; + + writer.writeAll( + \\ + \\Lightpanda has crashed. Please report the issue: + \\https://github.com/lightpanda-io/browser/issues + \\ + ) catch abort(); + + writer.print("\nreason: {s}\n", .{reason}) catch abort(); + writer.print("OS: {s}\n", .{@tagName(builtin.os.tag)}) catch abort(); + writer.print("mode: {s}\n", .{@tagName(builtin.mode)}) catch abort(); + writer.print("version: {s}\n", .{lp.build_config.git_commit}) catch abort(); + inline for (@typeInfo(@TypeOf(args)).@"struct".fields) |f| { + writer.writeAll(f.name ++ ": ") catch break; + @import("log.zig").writeValue(.pretty, @field(args, f.name), writer) catch abort(); + writer.writeByte('\n') catch abort(); + } + + std.debug.dumpCurrentStackTraceToWriter(begin_addr, writer) catch abort(); + } + + report(reason) catch {}; + }, + 1 => { + panic_level = 2; + var stderr_w = std.fs.File.stderr().writerStreaming(&.{}); + const stderr = &stderr_w.interface; + stderr.writeAll("panicked during a panic. Aborting.\n") catch abort(); + }, + else => {}, + }; + + abort(); +} + +fn report(reason: []const u8) !void { + if (@import("telemetry/telemetry.zig").isDisabled()) { + return; + } + + var curl_path: [2048]u8 = undefined; + const curl_path_len = curlPath(&curl_path) orelse return; + + var args_buffer: [4096]u8 = undefined; + var writer: std.Io.Writer = .fixed(&args_buffer); + + try writer.print("https://crash.lightpanda.io/?v={s}&r=", .{lp.build_config.git_commit}); + for (reason) |b| { + switch (b) { + 'A'...'Z', 'a'...'z', '0'...'9', '-', '.', '_' => try writer.writeByte(b), + ' ' => try writer.writeByte('+'), + else => try writer.writeByte('!'), // some weird character, that we shouldn't have, but that'll we'll replace with a weird (bur url-safe) character + } + } + + try writer.writeByte(0); + const url = writer.buffered(); + + var argv = [_:null]?[*:0]const u8{ + curl_path[0..curl_path_len :0], + "-fsSL", + url[0 .. url.len - 1 :0], + }; + std.debug.print("*{s}*\n", .{argv[2].?}); + + const result = std.c.fork(); + switch (result) { + 0 => { + _ = std.c.close(0); + _ = std.c.close(1); + _ = std.c.close(2); + _ = std.c.execve(argv[0].?, &argv, std.c.environ); + std.c.exit(0); + }, + else => return, + } +} + +fn curlPath(buf: []u8) ?usize { + const path = std.posix.getenv("PATH") orelse return null; + var it = std.mem.tokenizeScalar(u8, path, std.fs.path.delimiter); + + var fba = std.heap.FixedBufferAllocator.init(buf); + const allocator = fba.allocator(); + + const cwd = std.fs.cwd(); + while (it.next()) |p| { + defer fba.reset(); + const full_path = std.fs.path.joinZ(allocator, &.{ p, "curl" }) catch continue; + cwd.accessZ(full_path, .{}) catch continue; + return full_path.len; + } + return null; +} diff --git a/src/lightpanda.zig b/src/lightpanda.zig index f2cd5ac9..361d872d 100644 --- a/src/lightpanda.zig +++ b/src/lightpanda.zig @@ -27,6 +27,9 @@ pub const log = @import("log.zig"); pub const js = @import("browser/js/js.zig"); pub const dump = @import("browser/dump.zig"); pub const build_config = @import("build_config"); +pub const crash_handler = @import("crash_handler.zig"); + +const IS_DEBUG = @import("builtin").mode == .Debug; pub const FetchOpts = struct { wait_ms: u32 = 5000, @@ -69,6 +72,23 @@ pub fn fetch(app: *App, url: [:0]const u8, opts: FetchOpts) !void { try writer.flush(); } +pub inline fn assert(ok: bool, comptime ctx: []const u8, args: anytype) void { + if (!ok) { + if (comptime IS_DEBUG) { + unreachable; + } + assertionFailure(ctx, args); + } +} + +noinline fn assertionFailure(comptime ctx: []const u8, args: anytype) noreturn { + @branchHint(.cold); + if (@inComptime()) { + @compileError(std.fmt.comptimePrint("assertion failure: " ++ ctx, args)); + } + @import("crash_handler.zig").crash(ctx, args, @returnAddress()); +} + test { std.testing.refAllDecls(@This()); } diff --git a/src/main.zig b/src/main.zig index 19da7a3c..d0d83b56 100644 --- a/src/main.zig +++ b/src/main.zig @@ -24,6 +24,7 @@ const Allocator = std.mem.Allocator; const log = lp.log; const App = lp.App; const SigHandler = @import("Sighandler.zig"); +pub const panic = lp.crash_handler.panic; pub fn main() !void { // allocator diff --git a/src/telemetry/telemetry.zig b/src/telemetry/telemetry.zig index ebac6319..cffa8768 100644 --- a/src/telemetry/telemetry.zig +++ b/src/telemetry/telemetry.zig @@ -10,6 +10,10 @@ const Notification = @import("../Notification.zig"); const uuidv4 = @import("../id.zig").uuidv4; const IID_FILE = "iid"; +pub fn isDisabled() bool { + return std.process.hasEnvVarConstant("LIGHTPANDA_DISABLE_TELEMETRY"); +} + pub const Telemetry = TelemetryT(blk: { if (builtin.mode == .Debug or builtin.is_test) break :blk NoopProvider; break :blk @import("lightpanda.zig").LightPanda; @@ -30,7 +34,7 @@ fn TelemetryT(comptime P: type) type { const Self = @This(); pub fn init(app: *App, run_mode: App.RunMode) !Self { - const disabled = std.process.hasEnvVarConstant("LIGHTPANDA_DISABLE_TELEMETRY"); + const disabled = isDisabled(); if (builtin.mode != .Debug and builtin.is_test == false) { log.info(.telemetry, "telemetry status", .{ .disabled = disabled }); }