ReviewOS

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
11 days ago

Follow-up to #70. That issue's ask was the --log-file flag, which #78 delivered — but it also noted, separately, that the page-facing craft.log.* namespace was macOS-only and discarded its payload elsewhere. I didn't do that half, so here it is.

What it did

if (builtin.os.tag == .macos) {
    std.debug.print("[craft.{s}] {s}\n", .{ level, message });
}
bridge_error.sendResultToJS(self.allocator, "log", "{\"ok\":true}");

On Linux and Windows the payload was parsed, discarded, and answered {"ok":true}. A page that logged into that got a resolved promise and no record anywhere.

On macOS it went to stderr rather than through the host logger — so --log-file, which now exists, did not contain it either.

What it does now

Page records go through log.zig, the same sink --log-file configures, on every platform:

[2026-08-27T13:31:14Z] INFO [page] hello from the page
[2026-08-27T13:31:14Z] WARN [page] a warning from the page
[2026-08-27T13:31:14Z] ERROR [page] and an error

Verified by running the binary with --log-file --log-quiet and a page calling all four methods. The debug call is correctly absent — default level is info — and appears when --log-level debug is passed.

  • Tagged [page], because the host's records and the page's now share one stream and telling them apart is most of what makes that stream readable.
  • An unrecognised level is recorded as info, not dropped — silently discarding it would be the same failure this file already had.
  • The message is clamped, because it comes from the page.

The dead os_log handle

os_log_create("com.craft.app", "default") was called at init, stored, and never used. The comment beside it described a future revision reaching os_log properly through libBlocksRuntime — which never happened, and which a file sink is a better answer to. Removed rather than left as a placeholder for something that now has a better implementation.

Tests

The file had none, which is why nobody noticed two of three platforms recorded nothing. The level mapping is now a pure function with 5 tests, wired into zig build test — including that an invented level is still heard, and that the four levels the JS facade actually sends all map somewhere.

Verified

zig build · zig build test · zig build test-js · x86_64-windows cross-build · zig fmt --check · 511 bun tests · pickier 38 warnings, unchanged from main.

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.