From b9ddac878c8f20cfb87d3c32298cb9f677ee9d29 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 5 Feb 2026 17:35:15 +0800 Subject: [PATCH] Log unknown object properties in debug builds We currently log any unknown property access on the global (window). This has proven pretty useful when debugging, often letting us know a type we're completely missing. However, it isn't just about what types we have, it's also about the properties those types expose. Inspired by the wasted time spent on https://github.com/lightpanda-io/browser/pull/1473, this commit adds the same type of logging for unknown properties on objects. In debug mode only. This is only done for objects which don't have their own NamedIndexer. --- src/browser/js/Env.zig | 2 +- src/browser/js/Snapshot.zig | 51 +++++++++++++++++++++++++++++++++++ src/browser/js/bridge.zig | 54 ++++++++++++++++++++++++++++++++++++- 3 files changed, 105 insertions(+), 2 deletions(-) diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index cea7468a..7aa57410 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -142,7 +142,7 @@ pub fn init(app: *App, opts: InitOpts) !Env { const global_template_local = v8.v8__FunctionTemplate__InstanceTemplate(js_global).?; v8.v8__ObjectTemplate__SetNamedHandler(global_template_local, &.{ - .getter = bridge.unknownPropertyCallback, + .getter = bridge.unknownWindowPropertyCallback, .setter = null, .query = null, .deleter = null, diff --git a/src/browser/js/Snapshot.zig b/src/browser/js/Snapshot.zig index 445e80aa..7186fd7c 100644 --- a/src/browser/js/Snapshot.zig +++ b/src/browser/js/Snapshot.zig @@ -261,6 +261,19 @@ pub fn create() !Snapshot { }; } +// Helper to check if a JsApi has a NamedIndexed handler +fn hasNamedIndexedGetter(comptime JsApi: type) bool { + const declarations = @typeInfo(JsApi).@"struct".decls; + inline for (declarations) |d| { + const value = @field(JsApi, d.name); + const T = @TypeOf(value); + if (T == bridge.NamedIndexed) { + return true; + } + } + return false; +} + // Count total callbacks needed for external_references array fn countExternalReferences() comptime_int { @setEvalBranchQuota(100_000); @@ -301,6 +314,15 @@ fn countExternalReferences() comptime_int { } } + // In debug mode, add unknown property callbacks for types without NamedIndexed + if (comptime IS_DEBUG) { + inline for (JsApis) |JsApi| { + if (!hasNamedIndexedGetter(JsApi)) { + count += 1; + } + } + } + return count + 1; // +1 for null terminator } @@ -357,6 +379,16 @@ fn collectExternalReferences() [countExternalReferences()]isize { } } + // In debug mode, collect unknown property callbacks for types without NamedIndexed + if (comptime IS_DEBUG) { + inline for (JsApis) |JsApi| { + if (!hasNamedIndexedGetter(JsApi)) { + references[idx] = @bitCast(@intFromPtr(bridge.unknownObjectPropertyCallback(JsApi))); + idx += 1; + } + } + } + return references; } @@ -393,6 +425,7 @@ fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *v8.Functio const instance = v8.v8__FunctionTemplate__InstanceTemplate(template); const declarations = @typeInfo(JsApi).@"struct".decls; + var has_named_index_getter = false; inline for (declarations) |d| { const name: [:0]const u8 = d.name; @@ -453,6 +486,7 @@ fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *v8.Functio .flags = v8.kOnlyInterceptStrings | v8.kNonMasking, }; v8.v8__ObjectTemplate__SetNamedHandler(instance, &configuration); + has_named_index_getter = true; }, bridge.Iterator => { const function_template = @constCast(v8.v8__FunctionTemplate__New__DEFAULT2(isolate, value.func).?); @@ -490,6 +524,23 @@ fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *v8.Functio const js_value = v8.v8__String__NewFromUtf8(isolate, JsApi.Meta.name.ptr, v8.kNormal, @intCast(JsApi.Meta.name.len)); v8.v8__Template__Set(@ptrCast(instance), js_name, js_value, v8.ReadOnly + v8.DontDelete); } + + if (comptime IS_DEBUG) { + if (!has_named_index_getter) { + var configuration: v8.NamedPropertyHandlerConfiguration = .{ + .getter = bridge.unknownObjectPropertyCallback(JsApi), + .setter = null, + .query = null, + .deleter = null, + .enumerator = null, + .definer = null, + .descriptor = null, + .data = null, + .flags = v8.kOnlyInterceptStrings | v8.kNonMasking, + }; + v8.v8__ObjectTemplate__SetNamedHandler(instance, &configuration); + } + } } fn protoIndexLookup(comptime JsApi: type) ?bridge.JsApiLookup.BackingInt { diff --git a/src/browser/js/bridge.zig b/src/browser/js/bridge.zig index e761824c..9e4119a3 100644 --- a/src/browser/js/bridge.zig +++ b/src/browser/js/bridge.zig @@ -410,7 +410,7 @@ const Finalizer = struct { from_v8: *const fn (?*const v8.WeakCallbackInfo) callconv(.c) void, }; -pub fn unknownPropertyCallback(c_name: ?*const v8.Name, handle: ?*const v8.PropertyCallbackInfo) callconv(.c) u8 { +pub fn unknownWindowPropertyCallback(c_name: ?*const v8.Name, handle: ?*const v8.PropertyCallbackInfo) callconv(.c) u8 { const v8_isolate = v8.v8__PropertyCallbackInfo__GetIsolate(handle).?; var caller: Caller = undefined; caller.init(v8_isolate); @@ -471,6 +471,58 @@ pub fn unknownPropertyCallback(c_name: ?*const v8.Name, handle: ?*const v8.Prope return 0; } +// Only used for debugging +pub fn unknownObjectPropertyCallback(comptime JsApi: type) *const fn (?*const v8.Name, ?*const v8.PropertyCallbackInfo) callconv(.c) u8 { + if (comptime !IS_DEBUG) { + @compileError("unknownObjectPropertyCallback should only be used in debug builds"); + } + + return struct { + fn wrap(c_name: ?*const v8.Name, handle: ?*const v8.PropertyCallbackInfo) callconv(.c) u8 { + const v8_isolate = v8.v8__PropertyCallbackInfo__GetIsolate(handle).?; + + var caller: Caller = undefined; + caller.init(v8_isolate); + defer caller.deinit(); + + const local = &caller.local; + + var hs: js.HandleScope = undefined; + hs.init(local.isolate); + defer hs.deinit(); + + const property: []const u8 = js.String.toSlice(.{ .local = local, .handle = @ptrCast(c_name.?) }) catch { + return 0; + }; + + if (std.mem.startsWith(u8, property, "__")) { + // some frameworks will extend built-in types using a __ prefix + // these should always be safe to ignore. + return 0; + } + + if (JsApi == @import("../webapi/cdata/Text.zig").JsApi or JsApi == @import("../webapi/cdata/Comment.zig").JsApi) { + if (std.mem.eql(u8, property, "tagName")) { + // knockout does this, a lot. + return 0; + } + } + + const ignored = std.StaticStringMap(void).initComptime(.{}); + if (!ignored.has(property)) { + log.debug(.unknown_prop, "unknown object property", .{ + .object = if (@hasDecl(JsApi.Meta, "name")) JsApi.Meta.name else @typeName(JsApi), + .info = "but the property can exist in pure JS", + .stack = local.stackTrace() catch "???", + .property = property, + }); + } + // not intercepted + return 0; + } + }.wrap; +} + // Given a Type, returns the length of the prototype chain, including self fn prototypeChainLength(comptime T: type) usize { var l: usize = 1;