From 728f61a80d47b1f1aaa6168ef241ebaa4d092172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Tue, 22 Sep 2026 17:54:44 +0200 Subject: [PATCH] log: check the message rules at comptime logToErased asserts that a log message is at most 30 characters of plain text, but only in a debug build and only once the line actually runs. A message on a rare path therefore ships fine and then panics on whoever first reaches it: `serve --host 0.0.0.0` without --advertise-host crashed on startup in every debug build, because "advertising loopback for wildcard bind" is 38 characters. Every message is a literal, so make the six wrappers take a comptime msg and apply the same two rules through @compileError. The runtime check stays as the backstop for the paths the compiler does not analyse for the current target, and now reads the same constant. Eleven messages were over the limit; shorten them. The detail already lives in the kv pairs in each case. renderFailed takes its message as comptime now, the only call site that passed a runtime one. Note the check only covers code analysed for the target being built: the two in Certificates.zig sit in an OS switch prong that Linux never compiles, and were found by scanning the source rather than by the compiler. --- src/agent/auth/codex.zig | 2 +- src/browser/frame/observers.zig | 2 +- src/browser/js/Env.zig | 4 +- src/browser/screenshot.zig | 2 +- src/browser/webapi/Performance.zig | 2 +- .../webapi/ServiceWorkerGlobalScope.zig | 2 +- .../webapi/SharedWorkerGlobalScope.zig | 4 +- src/log.zig | 41 +++++++++++++++---- src/network/Certificates.zig | 4 +- src/server/cdp/domains/network.zig | 2 +- src/server/http.zig | 2 +- 11 files changed, 47 insertions(+), 20 deletions(-) diff --git a/src/agent/auth/codex.zig b/src/agent/auth/codex.zig index 01b0cff06..cdee87474 100644 --- a/src/agent/auth/codex.zig +++ b/src/agent/auth/codex.zig @@ -155,7 +155,7 @@ fn deviceLogin(allocator: std.mem.Allocator, interrupt: ?*zenai.http.Interrupt) const code_res = try post(a, interrupt, device_code_url, "application/json", "{\"client_id\":\"" ++ client_id ++ "\"}"); if (code_res.status != .ok) { - log.warn(.app, "codex device-code request failed", .{ .status = @intFromEnum(code_res.status), .body = code_res.body }); + log.warn(.app, "codex device-code failed", .{ .status = @intFromEnum(code_res.status), .body = code_res.body }); return error.DeviceCodeRequestFailed; } const dc = try std.json.parseFromSliceLeaky(DeviceCode, a, code_res.body, .{ .ignore_unknown_fields = true }); diff --git a/src/browser/frame/observers.zig b/src/browser/frame/observers.zig index fe1ff715a..6c1a8fb13 100644 --- a/src/browser/frame/observers.zig +++ b/src/browser/frame/observers.zig @@ -171,7 +171,7 @@ pub fn scheduleIntersectionChecks(frame: *Frame) void { frame._intersection.check_scheduled = true; frame.js.queueIntersectionChecks() catch |err| { frame._intersection.check_scheduled = false; - log.err(.frame, "frame.scheduleIntersectionChecks", .{ .err = err, .type = frame._type, .url = frame.url }); + log.err(.frame, "scheduleIntersectionChecks", .{ .err = err, .type = frame._type, .url = frame.url }); }; } diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index c3632b147..40118c7fc 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -443,7 +443,7 @@ pub fn hideServiceWorker(self: *const Env, comptime is_frame: bool, v8_context: var deleted: v8.MaybeBool = undefined; v8.v8__Object__Delete(global_obj, v8_context, @ptrCast(self.disabled_api_names.get(self.isolate.handle, "caches")), &deleted); if (deleted.has_value == false or deleted.value == false) { - log.warn(.js, "failed to hide experimental API", .{ .interface = "global", .member = "caches" }); + log.warn(.js, "experimental API not hidden", .{ .interface = "global", .member = "caches" }); } } @@ -457,7 +457,7 @@ fn deletePrototypeMember(self: *const Env, v8_context: *const v8.Context, global var deleted: v8.MaybeBool = undefined; v8.v8__Object__Delete(@ptrCast(prototype), v8_context, @ptrCast(names.get(isolate, member)), &deleted); if (deleted.has_value == false or deleted.value == false) { - log.warn(.js, "failed to hide experimental API", .{ .interface = interface, .member = member }); + log.warn(.js, "experimental API not hidden", .{ .interface = interface, .member = member }); } } diff --git a/src/browser/screenshot.zig b/src/browser/screenshot.zig index db7603773..2bd66ef0c 100644 --- a/src/browser/screenshot.zig +++ b/src/browser/screenshot.zig @@ -155,7 +155,7 @@ pub fn collect(arena: Allocator, state: RenderTree.State, frame: *Frame) ![]cons // Any non-zero rc means nothing usable was written, so the caller has to fail // rather than hand back a truncated file. WriteFailed is the only error // jsonStringify's signature can carry, hence the log line. -fn renderFailed(what: []const u8, rc: i32) error{WriteFailed} { +fn renderFailed(comptime what: []const u8, rc: i32) error{WriteFailed} { log.err(.browser, what, .{ .reason = switch (rc) { RC_WRITE_REFUSED => "write refused", RC_INVALID => "invalid options", diff --git a/src/browser/webapi/Performance.zig b/src/browser/webapi/Performance.zig index cc5e2f2b4..16ce09cf2 100644 --- a/src/browser/webapi/Performance.zig +++ b/src/browser/webapi/Performance.zig @@ -617,7 +617,7 @@ fn scheduleBufferFull(self: *Performance) !void { const exec = perf._exec; const event = try Event.initTrusted(.wrap(BUFFER_FULL), .{}, exec.page); try exec.dispatch(perf.asEventTarget(), event, perf._on_buffer_full, .{ - .context = "Performance.resourcetimingbufferfull", + .context = "Performance.bufferfull", }); return null; } diff --git a/src/browser/webapi/ServiceWorkerGlobalScope.zig b/src/browser/webapi/ServiceWorkerGlobalScope.zig index 7159ed883..0417624b9 100644 --- a/src/browser/webapi/ServiceWorkerGlobalScope.zig +++ b/src/browser/webapi/ServiceWorkerGlobalScope.zig @@ -404,7 +404,7 @@ fn dispatchExtendable( self._pending_event = event; errdefer self.releasePendingEvent(); - try wgs.dispatch(wgs.asEventTarget(), base, handler, .{ .context = "ServiceWorkerGlobalScope lifecycle" }); + try wgs.dispatch(wgs.asEventTarget(), base, handler, .{ .context = "service worker lifecycle" }); // Seal only after the handlers have run, so a synchronous waitUntil is // counted before an empty pending set can complete the phase. diff --git a/src/browser/webapi/SharedWorkerGlobalScope.zig b/src/browser/webapi/SharedWorkerGlobalScope.zig index 3c2547745..3e6ff0cb6 100644 --- a/src/browser/webapi/SharedWorkerGlobalScope.zig +++ b/src/browser/webapi/SharedWorkerGlobalScope.zig @@ -319,7 +319,7 @@ fn releaseScriptArena(self: *SharedWorkerGlobalScope) void { fn drainPendingConnects(self: *SharedWorkerGlobalScope) void { for (self._pending_connects.items) |port| { self.scheduleConnect(port) catch |err| { - log.warn(.browser, "shared worker drain connect failed", .{ .err = err }); + log.warn(.browser, "shared worker drain failed", .{ .err = err }); }; } self._pending_connects.clearRetainingCapacity(); @@ -382,7 +382,7 @@ const ConnectCallback = struct { .cancelable = false, }, wgs.page)).asEvent(); - try wgs.dispatch(target, event, on_connect, .{ .context = "SharedWorkerGlobalScope.connect" }); + try wgs.dispatch(target, event, on_connect, .{ .context = "shared worker connect" }); return null; } }; diff --git a/src/log.zig b/src/log.zig index b6ccf58dc..e487c1c14 100644 --- a/src/log.zig +++ b/src/log.zig @@ -140,27 +140,54 @@ pub const Format = enum { pretty, }; -pub fn debug(scope: Scope, msg: []const u8, data: anytype) void { +// A message is a short, plain-text key; the detail belongs in the kvs. +const max_msg_len = 30; + +/// The comptime form of the checks in logToErased. Those only fire in a debug +/// build *and* only once the line actually runs, so a message on a rare path +/// can ship and then panic on whoever first hits it. Every message below is a +/// literal, so the same rules can be a build error instead. +fn validateMsg(comptime msg: []const u8) void { + comptime { + if (msg.len > max_msg_len) { + @compileError("log msg cannot be more than 30 characters: " ++ msg); + } + for (msg) |b| { + switch (b) { + 'A'...'Z', 'a'...'z', ' ', '0'...'9', '_', '-', '.', '{', '}' => {}, + else => @compileError("log msg contains an invalid character: " ++ msg), + } + } + } +} + +pub fn debug(scope: Scope, comptime msg: []const u8, data: anytype) void { + comptime validateMsg(msg); log(scope, .debug, msg, data); } -pub fn info(scope: Scope, msg: []const u8, data: anytype) void { +pub fn info(scope: Scope, comptime msg: []const u8, data: anytype) void { + comptime validateMsg(msg); log(scope, .info, msg, data); } -pub fn warn(scope: Scope, msg: []const u8, data: anytype) void { +pub fn warn(scope: Scope, comptime msg: []const u8, data: anytype) void { + comptime validateMsg(msg); log(scope, .warn, msg, data); } -pub fn err(scope: Scope, msg: []const u8, data: anytype) void { +pub fn err(scope: Scope, comptime msg: []const u8, data: anytype) void { + comptime validateMsg(msg); log(scope, .err, msg, data); } -pub fn fatal(scope: Scope, msg: []const u8, data: anytype) void { +pub fn fatal(scope: Scope, comptime msg: []const u8, data: anytype) void { + comptime validateMsg(msg); log(scope, .fatal, msg, data); } -pub fn note(scope: Scope, msg: []const u8, data: anytype) void { +pub fn note(scope: Scope, comptime msg: []const u8, data: anytype) void { + comptime validateMsg(msg); if (comptime lp.IS_TEST == false) { log(scope, .note, msg, data); } @@ -236,7 +263,7 @@ fn logTo(scope: Scope, level: Level, msg: []const u8, data: anytype, out: *std.I fn logToErased(scope: Scope, level: Level, msg: []const u8, kvs: []const KV, out: *std.Io.Writer) !void { if (lp.IS_DEBUG) { - if (msg.len > 30) { + if (msg.len > max_msg_len) { std.debug.print("debug-only-panic: log msg cannot be more than 30 characters: {s}", .{msg}); @panic("invalid log msg"); } diff --git a/src/network/Certificates.zig b/src/network/Certificates.zig index b85658cc2..f33d35d84 100644 --- a/src/network/Certificates.zig +++ b/src/network/Certificates.zig @@ -102,14 +102,14 @@ fn storeFromSystemCA(allocator: Allocator) !*crypto.X509_STORE { // advances `ptr` past it, so we just hand it the rest of the buffer. var ptr: [*]const u8 = bytes.ptr + index.*; const x509 = crypto.d2i_X509(null, &ptr, @intCast(bytes.len - index.*)) orelse { - log.warn(.app, "Skipping unparseable system cert", .{}); + log.warn(.app, "Skipping unparseable cert", .{}); continue; }; defer crypto.X509_free(x509); // add_cert takes its own ref; drop ours. const result = crypto.X509_STORE_add_cert(store, x509); if (result != 1) { - log.warn(.app, "Failed to add X509 cert to store", .{}); + log.warn(.app, "Failed to add X509 cert", .{}); } count += 1; } diff --git a/src/server/cdp/domains/network.zig b/src/server/cdp/domains/network.zig index d239ab82b..b431d88bd 100644 --- a/src/server/cdp/domains/network.zig +++ b/src/server/cdp/domains/network.zig @@ -105,7 +105,7 @@ fn emulateNetworkConditions(cmd: *CDP.Command) !void { } // -1 disables a throughput limit. if (params.latency > 0 or params.downloadThroughput > 0 or params.uploadThroughput > 0) { - log.warn(.not_implemented, "Network.emulateNetworkConditions", .{ .param = "throttling" }); + log.warn(.not_implemented, "Network.emulateConditions", .{ .param = "throttling" }); } return cmd.sendResult(null, .{}); } diff --git a/src/server/http.zig b/src/server/http.zig index b2883a914..bf1bf9d80 100644 --- a/src/server/http.zig +++ b/src/server/http.zig @@ -1036,7 +1036,7 @@ pub fn buildJSONVersionResponse(app: *const App, port: u16) ![]const u8 { // advertiseHost() falls back to 127.0.0.1 so clients can still // connect locally. Surface the trade-off so users running // outside the same host know they have to opt in. - log.note(.cdp, "advertising loopback for wildcard bind", .{ + log.note(.cdp, "wildcard bind advertise", .{ .message = "--host is a wildcard (0.0.0.0 / ::) without --advertise-host; clients on other hosts will need --advertise-host to reach the CDP endpoint", }); }