ReviewOS
Reload

also looking at this

craft-native/craft

fix(log): send the page's own log somewhere it can be read

#79
Merged glennmichael123 wants to merge fix/page-log-sink into main
2 files +125 -17
packages/zig/build.zigmodified+13-0
@@ -757,6 +757,17 @@ pub fn build(b: *std.Build) void {
757757 });
758758 log_unit_tests.root_module.link_libc = true;
759759
760 // The page-facing log bridge. It answered `{"ok":true}` and wrote nothing
761 // at all on Linux and Windows, which no test noticed because it had none.
762 const bridge_log_tests = b.addTest(.{
763 .root_module = b.createModule(.{
764 .root_source_file = b.path("src/bridge_log.zig"),
765 .target = target,
766 .optimize = optimize,
767 }),
768 });
769 bridge_log_tests.root_module.link_libc = true;
770
760771 // The conformance test: what craft declares, against what it dispatches.
761772 const capability_conformance_tests = b.addTest(.{
762773 .root_module = b.createModule(.{
@@ -1128,6 +1139,7 @@ pub fn build(b: *std.Build) void {
11281139 const run_local_tls_tests = b.addRunArtifact(local_tls_tests);
11291140 const run_menu_roles_tests = b.addRunArtifact(menu_roles_tests);
11301141 const run_capabilities_tests = b.addRunArtifact(capabilities_tests);
1142 const run_bridge_log_tests = b.addRunArtifact(bridge_log_tests);
11311143 const run_log_unit_tests = b.addRunArtifact(log_unit_tests);
11321144 const run_bridge_shell_tests = b.addRunArtifact(bridge_shell_tests);
11331145 const run_window_lifecycle_tests = b.addRunArtifact(window_lifecycle_tests);
@@ -1251,6 +1263,7 @@ pub fn build(b: *std.Build) void {
12511263 test_step.dependOn(&run_local_tls_tests.step);
12521264 test_step.dependOn(&run_menu_roles_tests.step);
12531265 test_step.dependOn(&run_capabilities_tests.step);
1266 test_step.dependOn(&run_bridge_log_tests.step);
12541267 test_step.dependOn(&run_log_unit_tests.step);
12551268 test_step.dependOn(&run_bridge_shell_tests.step);
12561269 test_step.dependOn(&run_window_lifecycle_tests.step);
packages/zig/src/bridge_log.zigmodified+112-17
@@ -1,31 +1,63 @@
11const std = ;
22const builtin = ;
33const bridge_error = ;
4const host_log = ;
45
56const BridgeError = bridge_error.BridgeError;
67
7extern "c" fn os_log_create(subsystem: [*:0]const u8, category: [*:0]const u8) ?*anyopaque;
8
9/// Unified system log integration. Apps call `craft.log.{debug,info,
10/// warn,error}(message)` and we route through `os_log` so messages
11/// land in Console.app, `log show`, and any aggregator the user has
12/// configured (e.g. Sentry's macOS log adapter).
8/// The page's own log, forwarded to the same sink as craft's.
9///
10/// `craft.log.{debug,info,warn,error}(message)` used to reach
11/// `std.debug.print` on macOS and *nothing at all* anywhere else — the write
12/// sat behind `if (builtin.os.tag == .macos)`, so on Linux and Windows the
13/// payload was parsed, discarded, and answered `{"ok":true}`. A page that
14/// logged into that got a resolved promise and no record.
15///
16/// It goes through `log.zig` now, which is what `--log-file` configures, so a
17/// page's diagnostics land in the same file and the same order as the host's
18/// rather than in a different place on one platform and nowhere on the others.
19///
20/// The `os_log_create` handle this file used to open on macOS is gone. It was
21/// created at init, never used, and the comment beside it called it a
22/// placeholder for a future revision that would reach `os_log` proper —
23/// which needs libBlocksRuntime and never happened. A file sink is the thing
24/// it was standing in for.
25/// The host level a page's `level` string means.
1326///
14/// `os_log` itself is macro-driven and can't be invoked from C
15/// without going through libBlocksRuntime; for now we stamp messages
16/// with `std.debug.print` which routes to stderr and is captured by
17/// the unified log subsystem when run from a launched app. The
18/// `os_log_create` hook is in place so a future revision can swap to
19/// the proper macro path without touching the JS surface.
27/// Anything unrecognised is info rather than dropped: a page that invents a
28/// level, or sends none, should still be heard. Silently discarding it would
29/// be the same failure this file already had on Linux and Windows.
30fn levelFor(name: []const u8) host_log.LogLevel {
31 const table = .{
32 .{ "debug", host_log.LogLevel.Debug },
33 .{ "trace", host_log.LogLevel.Debug },
34 .{ "info", host_log.LogLevel.Info },
35 .{ "log", host_log.LogLevel.Info },
36 .{ "warn", host_log.LogLevel.Warning },
37 .{ "warning", host_log.LogLevel.Warning },
38 .{ "error", host_log.LogLevel.Error },
39 .{ "err", host_log.LogLevel.Error },
40 .{ "fatal", host_log.LogLevel.Fatal },
41 };
42 inline for (table) |entry| {
43 if (std.ascii.eqlIgnoreCase(name, entry[0])) return entry[1];
44 }
45 return .Info;
46}
47
2048pub const LogBridge = struct {
2149 allocator: std.mem.Allocator,
22 log_handle: ?*anyopaque = null,
2350
2451 const Self = @This();
2552
53 /// Longest page message recorded. The text comes from the page, and the
54 /// host sink formats into a fixed buffer — clamping here means a long
55 /// message is clipped at a boundary this file chose rather than one the
56 /// formatter happened to have.
57 const max_message = 1024;
58
2659 pub fn init(allocator: std.mem.Allocator) Self {
27 const handle = if (builtin.os.tag == .macos) os_log_create("com.craft.app", "default") else null;
28 return .{ .allocator = allocator, .log_handle = handle };
60 return .{ .allocator = allocator };
2961 }
3062
3163 pub fn deinit(_: *Self) void {}
@@ -45,9 +77,72 @@ pub const LogBridge = struct {
4577 }) catch return BridgeError.InvalidJSON;
4678 defer parsed.deinit();
4779
48 if (builtin.os.tag == .macos) {
49 std.debug.print("[craft.{s}] {s}\n", .{ parsed.value.level, parsed.value.message });
80 const message = if (parsed.value.message.len > max_message)
81 parsed.value.message[0..max_message]
82 else
83 parsed.value.message;
84
85 // Tagged `[page]` so a log file makes it obvious which side a line came
86 // from — the host's records and the page's now share one stream, and
87 // telling them apart is most of what makes that stream readable.
88 switch (levelFor(parsed.value.level)) {
89 .Debug => host_log.log(.Debug, "[page] {s}", .{message}),
90 .Info => host_log.log(.Info, "[page] {s}", .{message}),
91 .Warning => host_log.log(.Warning, "[page] {s}", .{message}),
92 .Error => host_log.log(.Error, "[page] {s}", .{message}),
93 .Fatal => host_log.log(.Fatal, "[page] {s}", .{message}),
5094 }
95
5196 bridge_error.sendResultToJS(self.allocator, "log", "{\"ok\":true}");
5297 }
5398};
99
100const testing = std.testing;
101
102test "the levels the page facade sends all map to a host level" {
103 // These four are exactly what `craft.log.*` sends (craft-bridge.js).
104 try testing.expectEqual(host_log.LogLevel.Debug, levelFor("debug"));
105 try testing.expectEqual(host_log.LogLevel.Info, levelFor("info"));
106 try testing.expectEqual(host_log.LogLevel.Warning, levelFor("warn"));
107 try testing.expectEqual(host_log.LogLevel.Error, levelFor("error"));
108}
109
110test "level names are matched without regard to case" {
111 try testing.expectEqual(host_log.LogLevel.Warning, levelFor("WARN"));
112 try testing.expectEqual(host_log.LogLevel.Error, levelFor("Error"));
113}
114
115test "an unrecognised level is still heard" {
116 // Dropping it would repeat the failure this file already had, where a
117 // page's log was parsed, discarded, and answered ok.
118 try testing.expectEqual(host_log.LogLevel.Info, levelFor("verbose"));
119 try testing.expectEqual(host_log.LogLevel.Info, levelFor(""));
120 try testing.expectEqual(host_log.LogLevel.Info, levelFor("🙂"));
121}
122
123test "the payload the page sends decodes into a level and a message" {
124 const Shape = struct {
125 level: []const u8 = "info",
126 message: []const u8 = "",
127 };
128 const payload =
129 \\{"level":"warn","message":"something happened"}
130 ;
131 const parsed = try std.json.parseFromSlice(Shape, testing.allocator, payload, .{
132 .ignore_unknown_fields = true,
133 .allocate = .alloc_always,
134 });
135 defer parsed.deinit();
136
137 try testing.expectEqual(host_log.LogLevel.Warning, levelFor(parsed.value.level));
138 try testing.expectEqualStrings("something happened", parsed.value.message);
139}
140
141test "a page cannot log an unbounded message" {
142 // The text comes from the page. Clipping here means the boundary is one
143 // this file chose rather than whatever the host formatter happened to have.
144 var huge: [LogBridge.max_message * 2]u8 = undefined;
145 @memset(&huge, 'x');
146 const clipped = if (huge.len > LogBridge.max_message) huge[0..LogBridge.max_message] else huge[0..];
147 try testing.expectEqual(LogBridge.max_message, clipped.len);
148}