From f0916d58e87d76f11e30ee2455e50e4b22a3862d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 1 Sep 2024 10:29:53 -0700 Subject: [PATCH] crash: try to attach dimensions to the crash report --- pkg/sentry/main.zig | 8 +++++ pkg/sentry/value.zig | 26 +++++++++++++++ src/crash/sentry.zig | 78 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+) diff --git a/pkg/sentry/main.zig b/pkg/sentry/main.zig index 94821470b..35fc57dfe 100644 --- a/pkg/sentry/main.zig +++ b/pkg/sentry/main.zig @@ -14,6 +14,14 @@ pub fn captureEvent(value: Value) ?UUID { return uuid; } +pub fn setContext(key: []const u8, value: Value) void { + c.sentry_set_context_n(key.ptr, key.len, value.value); +} + +pub fn removeContext(key: []const u8) void { + c.sentry_remove_context_n(key.ptr, key.len); +} + pub fn setTag(key: []const u8, value: []const u8) void { c.sentry_set_tag_n(key.ptr, key.len, value.ptr, value.len); } diff --git a/pkg/sentry/value.zig b/pkg/sentry/value.zig index 47fbec9fa..dc200ce3b 100644 --- a/pkg/sentry/value.zig +++ b/pkg/sentry/value.zig @@ -24,6 +24,22 @@ pub const Value = struct { ) }; } + pub fn initObject() Value { + return .{ .value = c.sentry_value_new_object() }; + } + + pub fn initString(value: []const u8) Value { + return .{ .value = c.sentry_value_new_string_n(value.ptr, value.len) }; + } + + pub fn initBool(value: bool) Value { + return .{ .value = c.sentry_value_new_bool(@intFromBool(value)) }; + } + + pub fn initInt32(value: i32) Value { + return .{ .value = c.sentry_value_new_int32(value) }; + } + pub fn decref(self: Value) void { c.sentry_value_decref(self.value); } @@ -35,4 +51,14 @@ pub const Value = struct { pub fn isNull(self: Value) bool { return c.sentry_value_is_null(self.value) != 0; } + + /// sentry_value_set_by_key_n + pub fn setByKey(self: Value, key: []const u8, value: Value) void { + _ = c.sentry_value_set_by_key_n( + self.value, + key.ptr, + key.len, + value.value, + ); + } }; diff --git a/src/crash/sentry.zig b/src/crash/sentry.zig index 9d3b0eec3..6b649e713 100644 --- a/src/crash/sentry.zig +++ b/src/crash/sentry.zig @@ -7,6 +7,7 @@ const sentry = @import("sentry"); const internal_os = @import("../os/main.zig"); const crash = @import("main.zig"); const state = &@import("../global.zig").state; +const Surface = @import("../Surface.zig"); const log = std.log.scoped(.sentry); @@ -14,6 +15,22 @@ const log = std.log.scoped(.sentry); /// handling is a global process-wide thing. var init_thread: ?std.Thread = null; +/// Thread-local state that can be set by thread main functions so that +/// crashes have more context. +/// +/// This is a hack over Sentry native SDK limitations. The native SDK has +/// one global scope for all threads and no support for thread-local scopes. +/// This means that if we want to set thread-specific data we have to do it +/// on our own in the on crash callback. +pub const ThreadState = struct { + /// The surface that this thread is attached to. + surface: *Surface, +}; + +/// See ThreadState. This should only ever be set by the owner of the +/// thread entry function. +threadlocal var thread_state: ?ThreadState = null; + /// Process-wide initialization of our Sentry client. /// /// This should only be called from one thread, and deinit should be called @@ -70,6 +87,10 @@ fn initThread(gpa: Allocator) !void { ); sentry.c.sentry_options_set_transport(opts, @ptrCast(transport)); + // Set our crash callback. See beforeSend for more details on what we + // do here and why we use this. + sentry.c.sentry_options_set_before_send(opts, beforeSend, null); + // Determine the Sentry cache directory. const cache_dir = try internal_os.xdg.cache(alloc, .{ .subdir = "ghostty/sentry" }); sentry.c.sentry_options_set_database_path_n( @@ -109,6 +130,63 @@ pub fn deinit() void { _ = sentry.c.sentry_close(); } +fn beforeSend( + event_val: sentry.c.sentry_value_t, + _: ?*anyopaque, + _: ?*anyopaque, +) callconv(.C) sentry.c.sentry_value_t { + // The native SDK at the time of writing doesn't support thread-local + // scopes. The full SDK has one global scope. So we use the beforeSend + // handler to set thread-specific data such as window size, grid size, + // etc. that we can use to debug crashes. + + const thr_state = thread_state orelse { + // If we don't have thread state we note this in the context + // so we can see that but don't do anything else. + const obj = sentry.Value.initObject(); + errdefer obj.decref(); + obj.setByKey("unknown", sentry.Value.initBool(true)); + sentry.setContext("surface", obj); + return event_val; + }; + + // Read the surface data. This is likely unsafe because on a crash + // other threads can continue running. We don't have race-safe way to + // access this data so this might be corrupted but it's most likely fine. + { + const obj = sentry.Value.initObject(); + errdefer obj.decref(); + const surface = thr_state.surface; + obj.setByKey( + "screen-width", + sentry.Value.initInt32(std.math.cast(i32, surface.screen_size.width) orelse -1), + ); + obj.setByKey( + "screen-height", + sentry.Value.initInt32(std.math.cast(i32, surface.screen_size.height) orelse -1), + ); + obj.setByKey( + "grid-columns", + sentry.Value.initInt32(std.math.cast(i32, surface.grid_size.columns) orelse -1), + ); + obj.setByKey( + "grid-rows", + sentry.Value.initInt32(std.math.cast(i32, surface.grid_size.rows) orelse -1), + ); + obj.setByKey( + "cell-width", + sentry.Value.initInt32(std.math.cast(i32, surface.cell_size.width) orelse -1), + ); + obj.setByKey( + "cell-height", + sentry.Value.initInt32(std.math.cast(i32, surface.cell_size.height) orelse -1), + ); + sentry.setContext("dimensions", obj); + } + + return event_val; +} + pub const Transport = struct { pub fn send(envelope: *sentry.Envelope, ud: ?*anyopaque) callconv(.C) void { _ = ud;